diff --git a/docs/development/loopx-issue-fix-integration.md b/docs/development/loopx-issue-fix-integration.md new file mode 100644 index 0000000000..b1f933578b --- /dev/null +++ b/docs/development/loopx-issue-fix-integration.md @@ -0,0 +1,429 @@ +# BitFun × LoopX issue-fix 集成设计 + +状态：设计草案（未实现） +BitFun 基线：`eb6e9de5a17dcdfd972e8406cc25d1cf0a5996b0` +LoopX 基线：`b1c09f32`（editable 安装，`C:\codeagent\loopx`） +验证日期：2026-07-31 + +--- + +## 1. 目标与范围 + +给 BitFun 新增「自动修复某个仓库的 issue」能力。LoopX 只提供 `issue-fix` +这一条能力作为**决策骨架**，不引入 LoopX 的 quota / scheduler / todo 体系。 + +范围内： +- 枚举目标仓库的开放 issue，逐个走「判断 → 定位 → 修 → 验证 → PR」 +- 每个决策点由 LoopX 投影，每个证据由 BitFun 提供 +- 节奏、预算、人工门禁由 BitFun 现有 `thread_goal` 承担 + +范围外（本期不做）： +- LoopX 的 `quota` / `scheduler` / `todo` / 多 agent 协作 +- LoopX 的 reviewer 通知、Lark 集成、metrics +- 自动 merge + +--- + +## 2. 职责边界 + +四层，每层只拥有自己那部分： + +| 层 | 拥有 | 不拥有 | +|---|---|---| +| BitFun `thread_goal` | 何时跑下一轮、预算、人工门禁、终止 | 单个 issue 怎么修 | +| BitFun coding agent | 读代码、定位、改码、跑验证 | 该不该发 PR | +| LoopX `issue-fix` | 路线选择、PR 生命周期投影、就绪门禁 | 任何写操作、任何代码能力 | +| BitFun `review_platform` | 抓 issue、发 PR、鉴权 | 决策 | + +核心不变式：**LoopX 只做判断，不动手；BitFun 只提供证据，不自己决定路线。** + +LoopX 全程 `external_writes_performed: False`。它对缺失的信息不猜测，而是标为 +`unresolved_required_aspects` 并给出 `expert_next_action: read_repository_sources` +——这正是交还给 BitFun 的信号。 + +--- + +## 3. 已验证的链路 + +以下均在 BitFun 仓库 + 真实 issue #1849 上实测通过（只读，无副作用）。 + +### 3.1 主链路 + +``` +workflow-plan + ↓ unresolved: [change_scope, reproduction, validation] + ↓ expert_next_action: read_repository_sources +BitFun 读代码，产出 repository_context_json + ↓ +feasibility + ↓ repository_context_status: grounded + ↓ route: fix_pr | comment_only | triage_only +caller-repo-branch（--execute 才动仓库） + ↓ branch_action / branch_ready / validation_passed + ↓ review_packet.ready +pr-lifecycle + ↓ decision: runnable_successor | monitor_continuation | user_gate | no_followup +``` + +### 3.2 feasibility 的判断力（三组对照） + +同一 issue，只改输入： + +| 证据 + 复现 + 范围 | route | transition | +|---|---|---| +| grounded / confirmed / bounded | `fix_pr` | `runnable_successor` | +| 无 / missing / uncertain | `triage_only` | `no_followup` | +| grounded / confirmed / **oversized** | `triage_only` | `no_followup` | + +第三组是关键闸门：证据齐全且可复现，但范围过大时仍拒绝发 PR。 + +### 3.2.1 真正的 PR 门禁是 `--validation-label`，不是 context grounding + +实测（`loopx_issue_fix_contracts.rs` 两个互为对照的测试）： + +| context | `--validation-label` | route | +|---|---|---| +| **grounded** | 缺失 | `triage_only` | +| **partial**（validation 未覆盖） | 已提供 | **`fix_pr`** | + +结论：LoopX 区分两件事—— +- **context 里的 validation source** = 「你读了哪些测试文件」，影响 `coverage.validation` 和 + `context_status`，但**不**单独决定 route +- **`--validation-label`** = 「你打算怎么验证这个修复」，**这才是发 PR 的硬门禁** + +所以 BitFun 侧必须能说出验证手段（例如「web-ui focused vitest」），说不出就只能走 triage， +即使代码读得再透。反之，context 只是 partial 时仍可发 PR，只会带上 +`repository_context_partial` 这条 reason code。 + +### 3.2.2 什么算 aspect 已 grounded + +LoopX 的判定（`repository_context.py:165-169`），三个条件全满足才算： + +- `freshness == "current"`（且 context 必须带 `repository_revision`） +- `trust ∈ {authoritative, verified}` +- `source_kind != "external_expert"` + +`context_status` 则看 `change_scope` / `reproduction` / `validation` 三项：全 grounded → +`grounded`，部分 → `partial`，全无 → `ungrounded`。 + +`RepositoryContextBuilder::context_status()` 在本地复刻了这套判定，可在不启动子进程的 +情况下预测结果；契约测试逐 aspect 比对两者，防止规则漂移。 + +### 3.3 pr-lifecycle 的四种投影 + +| PR 状态 | decision | state_bucket | +|---|---|---| +| OPEN + 检查通过 | `monitor_continuation` | `review_required` | +| OPEN + CHANGES_REQUESTED + 检查失败 | `runnable_successor` | `checks_failed` | +| MERGED | `no_followup` | `terminal` | +| CLOSED | `no_followup` | `terminal` | + +维护者反馈（`maintainer-correction-json`）的四种 `correction_kind`： + +| correction_kind | decision | role | +|---|---|---| +| `actionable_patch` | `runnable_successor` | agent | +| `semantic_ambiguity` | **`user_gate`** | user | +| `missing_authority` | **`user_gate`** | user | +| `unchanged` | `monitor_continuation` | agent | + +两种 `user_gate` 是必须停下来问人的边界：**意图有歧义**、**缺写权限**。 + +### 3.4 修复循环本身 + +`repo-branch-fixture` 在临时 git repo 中跑完 branch → repro → patch → validation → PR evidence： +`ok: True`、`validated_fix_artifact_ready: true`、5 个 git 步骤逐条带 exit code。 + +--- + +## 4. 环境约束（Windows） + +### 4.1 必须设 `PYTHONUTF8=1` + +LoopX 包内 **123 处** `subprocess` 调用带 `text=True` 但不带 `encoding`，无一例外， +且无统一包装函数。在非 UTF-8 locale 的 Windows（本机 `cp936` / `gbk`）上， +`gh` 的 UTF-8 输出会以 GBK 解码而抛 `UnicodeDecodeError`。 + +`PYTHONUTF8=1` 把 `locale.getpreferredencoding()` 全局改为 UTF-8，一次覆盖全部 123 处， +零源码改动，已实测 `--fetch-metadata` 恢复正常。 + +**这是宿主职责**：BitFun spawn LoopX 进程时必须在 env 中带上该变量。 +不要试图修改 LoopX 源码——散弹改 123 处会与上游 `git pull` 冲突。 + +### 4.2 已知缺陷：临时目录清理与 Windows validation 启动 + +`repo-branch-fixture` 的 `finally` 清理会因 git object 只读属性抛 `WinError 5` +（`acceptance_loop.py:227` 的 `_remove_temporary_git_workspace` 重试 5 次无效—— +只读属性不是文件锁，等待不会改变结果）。 + +循环主体不受影响（实测 `ok: True`）。若 BitFun 要用这个子命令，需要： +- 要么在调用侧接受非零退出但解析已产出的 artifact +- 要么向上游提 `shutil.rmtree(onexc=...)` + `os.chmod(p, stat.S_IWRITE)` 的修复 + +另一个 Windows 缺陷已被 BitFun 侧修复：`caller-repo-branch --execute` 的 +validation command 由 LoopX 用 `subprocess.run(shlex.split(cmd))` 启动，不带 shell。 +Windows 上 `CreateProcess` 无法直接解析 `.cmd` / `.bat` shim（如 `pnpm`），报 +`[WinError 2]`。BitFun 编排器在 Windows 上自动用 `cmd /c` 包装 validation command +（`orchestrator.rs` 的 `windows_safe_validation_command`），这是宿主职责，不改 +LoopX 源码；2026-08-01 已在真实仓库实测通过。 + +**注**：memory 中记录的「pnpm WinError 2 阻塞」与 LoopX 无关——全仓仅两处提及 +pnpm，均在 benchmark 的正则字符串内，不执行。该记录需更正。 + +--- + +## 5. BitFun 侧改动 + +### 5.1 已有、可直接复用 + +| 能力 | 位置 | +|---|---| +| 取单个 issue（GitHub + GitLab） | `review_platform.rs:1077` `issue()` | +| issue 证据获取 | `review_platform.rs:3876` `acquire_issue_evidence` | +| issue 指纹 | `review_platform.rs:6336` `issue_fingerprint` | +| 创建 PR | `review_platform.rs:1224`；core 层封装 `service/review_platform/mod.rs:281` | +| 鉴权（含 `GhCli`） | `review_platform.rs` `load_stored_tokens` | +| 持久化目标循环 + 自动续跑 | `thread_goal.rs:580` `continuation_after_turn` | +| 预算与状态机 | `thread_goal.rs:647` `apply_budget_status` | +| 外部二进制探测先例 | `workspace_search/service.rs:660` `which::which` | +| git 操作 | `git2`（已是依赖） | + +### 5.2 已实现（后端） + +**A. issue 枚举** — `review_platform.rs`，提交 `8143f8ebc` + +```rust +pub async fn list_issues( + &self, + request: ReviewPlatformListIssuesRequest<'_>, +) -> Result; +``` + +用请求结构体而非位置参数，因为同级 `issue()` 已达 clippy 参数上限。返回轻量 +`ReviewPlatformIssueSummary`（不含 body 与评论），避免列举时拉取巨量数据。 + +provider 差异：GitHub 走 `gh` CLI、issues 端点会混入 PR（按 `pull_request` 字段过滤）、 +无 Link 头故以满页推断翻页；GitLab 走 HTTP、用项目内 `iid`、无 `all` 字面量（须省略 +参数）、翻页看 `x-next-page`。 + +**B. repository context 生成** — `loopx_issue_fix/repository_context.rs` + +`RepositoryContextBuilder` 在 `push` 时逐条校验，而非 build 时一次性报错——调用方能 +知道是哪条 source 有问题。已强制的 LoopX 约束： + +- `source_id` 形状 `^[A-Za-z0-9][A-Za-z0-9_.:-]{0,119}$`、不可重复 +- `reference` ≤260 字符、**禁绝对路径与 `..`**、URL 须 https 且无 query、 + Windows 分隔符归一化为 POSIX +- `summary` ≤220 字符、空白折叠后计数（与 LoopX 一致） +- sources ≤16 条 +- `memory_retrieval` / `external_expert` 的 trust 必须 `advisory` +- `freshness: current` 必须有 `repository_revision` + +另提供 `context_status()` / `ungrounded_required_aspects()`，本地复刻 LoopX 的 grounding +判定（见 3.2.2），可在不启动子进程的情况下预测结果并决定还需读什么。 + +**C. LoopX 进程调用层** — `loopx_issue_fix/mod.rs`，提交 `d43576b09` + +```rust +impl LoopxIssueFix { + /// None → 特性不可用，隐藏入口 + pub fn probe() -> Option; // LOOPX_BIN 覆盖，否则 which::which("loopx") + + pub async fn issue_fix(&self, args: I) + -> Result; + // 自动附加 issue-fix 前缀、--format json、env PYTHONUTF8=1 +} +``` + +关键实现细节：LoopX 的业务拒绝**同时**输出 `{"ok": false, "error": ...}` 到 stdout +**并**以非零码退出。因此必须先解析 stdout，否则结构化原因会被裸退出码覆盖——这是 +实测发现的，不是设计推断。 + +统一走 `--format json`，不解析 markdown。 + +### 5.3 挂载到 thread_goal + +一个 issue 一轮。`continuation_after_turn` 已实现自动续跑与上限 +（`MAX_THREAD_GOAL_AUTO_CONTINUATIONS = 100`，见 `runtime-ports/src/lib.rs:1671`）。 + +LoopX 的 decision 映射到 `ThreadGoalStatus`： + +| LoopX decision | BitFun 行为 | +|---|---| +| `runnable_successor` | 继续本轮工作 | +| `monitor_continuation` | 本 issue 完成，转下一个 | +| `user_gate` | `ThreadGoalStatus::Blocked`，等人 | +| `no_followup` | 本 issue 终态，转下一个 | + +`user_gate` → `Blocked` 是安全默认：`thread_goal_status_is_resumable()` +已允许人工恢复（`thread_goal.rs:321`）。 + +--- + +## 6. 产品形态 + +### 6.1 用户怎么用 + +**入口**：聊天头部一个图标按钮 → 右侧面板打开新页签（仿 +`FlowChatHeader.tsx:883-887` 的 PR 按钮 → `createReviewPlatformTab`）。 + +**布局**：左列 issue 列表带勾选框，右列当前 issue 的详情与进度。 + +``` +┌─ Issues ──────┬─ #1805 ──────────────┐ +│ ☑ #1677 ✓ │ 切换轮次无法跳转 │ +│ ☑ #1849 ✓ │ │ +│ ☑ #1805 ⟳ │ route: fix_pr │ +│ ☑ #1920 ⚠ │ 分支: codex/1805-fix │ +│ ☐ #1687 │ 验证: 进行中... │ +│ ☐ #1234 │ │ +│ │ 改动 3 个文件： │ +│ [全选] [开始] │ SessionsSection.tsx │ +└──────────────┴────────────────────────┘ +``` + +**流程**：打开页签 → 自动枚举开放 issue → 用户勾选要修哪些 → 点「开始」→ +先逐个跑只读的 `feasibility` 判定路线 → `fix_pr` 的 issue 作为一条任务消息 +提交给会话的 agent（`issue_fix_execute` → `submit_dialog_turn`），模型的流式 +输出直接出现在聊天区 → 其余路线记录原因码后转下一个 → 遇 `user_gate` 停下 +等确认。 + +**执行方式**：修复动作不是 BitFun 自己写代码，而是把任务交给 BitFun 现有 +的 agent 循环（与用户手动发消息完全同一条调度路径）：agent 读代码、定位、 +改码、跑验证，全部可见于聊天区。LoopX 在这一步只负责「该不该修」的判断 +（`feasibility` 返回 `fix_pr` 才会提交），不参与修的过程。 + +**四种行状态**，直接对应 LoopX 的 decision： + +| 行状态 | 图标 | 来源 | +|---|---|---| +| 排队中 | `○` | 已勾选未开始 | +| 正在修 | `⟳` | `runnable_successor` | +| 已完成 | `✓` | `monitor_continuation` / `no_followup` | +| 等你确认 | `⚠` | **`user_gate`** | + +### 6.2 复用哪些现成的东西 + +| 需要的 | 复用 | +|---|---| +| 逐项状态列表（勾选 + 进行中 + 完成 + 锁定） | `RemediationSelectionPanel.tsx:166-200`，两个 `Set` 驱动：`completedRemediationIds` / `fixingRemediationIds` | +| 分组、全选三态、`needs_decision` 展开选项 | 同上，`GROUP_PRIORITY_META` | +| 右侧面板页签的开启方式 | `tabUtils.ts:285-309` `createReviewPlatformTab` 的事件派发模式 | +| 面板容器与 PR 详情三页签 | `ReviewPlatformPanel.tsx`（2646 行） | +| 目标的暂停 / 恢复 / 终止 | `thread_goal` 现有状态机 | + +一个 issue 映射成一个 remediation item，`RemediationSelectionPanel` 的 +交互模型几乎可直接迁移，包括 `user_gate` 对应它已有的 `requiresDecision` 流程。 + +### 6.3 不做的 + +- **不加 token 预算输入框**。靠现有 `MAX_THREAD_GOAL_AUTO_CONTINUATIONS = 100` + 上限（`runtime-ports/src/lib.rs:1671`）与用户随时可暂停。BitFun 目前没有任何 + 预算输入 UI——`AgentAPI.ts:573-578` 的激活接口无该参数（后端 + `create_thread_goal` 支持，但仅 agent 工具可设），本期不新增这第一个。 +- **不加斜杠命令**。入口只有面板按钮一处。 +- **不做 MiniApp 版本**。 + +### 6.4 需要新增的前端 + +- 新 `PanelContentType` 成员（`panels/base/types.ts:20-36` 的联合类型） +- `FlexiblePanel.tsx:821` 的 switch 分支 +- 面板组件本体 + `createIssueFixTab`（仿 `tabUtils.ts:285`） +- `FlowChatHeader` 一个图标按钮 +- i18n 三语言键（`scripts/i18n-contract.test.mjs` 强制校验对齐） + +--- + +## 7. 权限与门禁 + +已确认的授权决定：**受限的常驻发 PR 权限**（GCWing/BitFun 为 1381 star 公开仓库）。 + +### 7.1 三层开关 + +三层互相独立，职责不同： + +| 层 | 机制 | 本期决定 | +|---|---|---| +| 编译期 | Cargo feature，非 `default` | **加一个 feature**，仿 `services-integrations/Cargo.toml:96` 的 `announcement` / `browser-control`（该 crate 为 `default = []`） | +| 能力探测 | `LoopxIssueFix::probe() -> Option` | 必需。仿 `workspace_search/service.rs:660` 的 `which::which`。`None` → 隐藏头部按钮 | +| 运行时 | 用户设置 bool | **不设默认关的总开关**：feature 编进去且 probe 成功即可用 | + +编译期决定「代码是否进二进制」，探测决定「运行环境是否具备」，两者都通过就可用。 + +### 7.2 「默认开」带来的后果 + +因为没有「默认关」的运行时总开关兜底，**发 PR 这一步的门禁必须落在动作本身**， +不能依赖用户没打开开关。具体要求： + +- `create_pull_request`（`review_platform.rs:1224`）的调用点必须在 `feasibility` + 返回 `route: fix_pr` **且** review packet `ready: true` 时才触发；两个条件缺一不可 +- 首次对某个仓库执行 `caller-repo-branch --execute` 需人工确认（会真实建分支） +- `pr-lifecycle` 返回 `user_gate` 时必须转 `ThreadGoalStatus::Blocked`， + 并在列表行上显示为 `⚠ 等你确认`，不得自动跨过 +- Cargo feature 在首个版本可以先不加入 `product-full`，让代码落地但不进发布构建， + 等真实仓库验证通过再纳入 + +必须保持人工的： +- LoopX 返回 `user_gate` 的两种情形（意图歧义、缺权限） +- merge（本期完全不做） +- `caller-repo-branch --execute` 首次在真实仓库建分支 + +可自动的： +- `workflow-plan` / `feasibility` / `pr-lifecycle`（只读投影，零写入） +- 分支内的改码与验证 +- 在 `feasibility` 返回 `fix_pr` 且 `review_packet.ready` 为真时发 PR + +--- + +## 8. 尚未验证 + +写实现前应补的： + +1. `caller-repo-branch --execute` 未在真实仓库跑过（会真的建分支） +2. `create_pull_request` 未与 LoopX 的 review packet 串联验证 +3. 多 issue 串行时 `thread_goal` 的 token 记账行为 +4. GitLab 路径完全未测（`map_gitlab_issue` 存在但未走过本链路） +5. `promote-discovered-issue`（agent 自己发现的缺陷）未纳入本期 + +--- + +## 9. 实现进度 + +后端优先，UI 最后——前四步都无外部副作用，可独立验证。 + +- [x] **1.** Cargo feature `loopx-issue-fix`（非 `default`，暂不在 `product-full`） +- [x] **2.** `LoopxIssueFix::probe()` + `issue_fix()`（含 `PYTHONUTF8=1`） +- [x] **3.** `list_issues()` +- [x] **4.** repository context 生成器 +- [x] **5.** 单 issue 编排器（`orchestrator.rs`，类型化 outcome） +- [x] **6.** 桌面 API 暴露（core facade → Tauri 命令 → 前端绑定） +- [x] **7.** 面板 UI（`panels/issue-fix/`，头部按钮，三语言） +- [x] **8.** `thread_goal` 桥接（`thread_goal_bridge.rs`，多 issue 串行） +- [x] **9.** 真实仓库验证通过，feature 已纳入 `product-full` + +### 测试覆盖 + +| 类型 | 数量 | 说明 | +|---|---|---| +| Rust 单元 | 53 | 含 issue 枚举映射、context 校验、编排器解析、goal 桥接、Windows validation 包装 | +| Rust 契约 | 12 | 驱动真实 loopx CLI，无 loopx 时优雅跳过 | +| Rust `#[ignore]` | 1 | 驱动真实 `gh` CLI 验证 issue 枚举 | +| 前端单元 | 29 | 行状态映射，重点是 `user_gate` 不被当作完成 | +| i18n 契约 | 37 | 三语言对齐 + 治理预算 | + +### 第 9 步的验证记录（2026-08-01） + +在真实公开仓库 GCWing/BitFun 上对 issue #1849 走完整链路： + +1. `workflow-plan --fetch-metadata`（只读）→ `candidate_runnable: true`，零写入 +2. `feasibility`（grounded context + confirmed repro + 命名 validation 面） + → `route: fix_pr`，四个 reason code 全部满足，`decision: runnable_successor` +3. `caller-repo-branch --execute`（经授权的临时 worktree `loopx/1849-verify`） + → 分支创建 + 真实 vitest validation 通过，`review_packet.ready: true` +4. 过程中发现并修复 Windows 缺陷：validation command 必须以 `cmd /c` 包装 + （见 4.2） + +按文档 7.2 的门禁，发 PR 动作需显式授权后才执行；feature 的编译期门禁已 +从「暂不入 product-full」切换为「纳入 product-full」。边界检查 +（`scripts/core-boundaries/rules/feature-rules.mjs`）同步登记了 +`loopx-issue-fix` 的 owner 覆盖与 product-full 组。 diff --git a/scripts/core-boundaries/cargo-dependency-boundaries.mjs b/scripts/core-boundaries/cargo-dependency-boundaries.mjs index 2287d11cd2..9693571001 100644 --- a/scripts/core-boundaries/cargo-dependency-boundaries.mjs +++ b/scripts/core-boundaries/cargo-dependency-boundaries.mjs @@ -135,6 +135,7 @@ const SERVICES_INTEGRATIONS_TOKIO_FEATURES = new Map([ ['miniapp-market', ['fs', 'io-util', 'net', 'process', 'rt', 'sync', 'time']], ['plugin-source', ['fs', 'rt', 'sync', 'time']], ['hook-import', ['fs', 'sync']], + ['loopx-issue-fix', ['fs', 'io-util', 'macros', 'process', 'sync']], ['remote-connect', ['fs', 'io-util', 'net', 'process', 'rt', 'sync', 'time']], ['remote-ssh', ['fs', 'io-util', 'macros', 'net', 'process', 'rt', 'sync', 'time']], ['remote-ssh-concrete', ['fs', 'io-util', 'macros', 'net', 'process', 'rt', 'sync', 'time']], diff --git a/scripts/core-boundaries/rules/feature-rules.mjs b/scripts/core-boundaries/rules/feature-rules.mjs index 3a1af0e3a7..6b70b479ca 100644 --- a/scripts/core-boundaries/rules/feature-rules.mjs +++ b/scripts/core-boundaries/rules/feature-rules.mjs @@ -128,7 +128,7 @@ export const optionalDependencyFeatureOwnerRules = [ { depName: 'anyhow', ownerFeatures: ['browser-control', 'debug-log', 'mcp', 'remote-connect', 'remote-ssh', 'remote-ssh-concrete'] }, { depName: 'async-trait', - ownerFeatures: ['git', 'mcp', 'remote-connect', 'remote-ssh', 'remote-ssh-concrete', 'review-platform', 'script-tool-runtime', 'speech', 'workspace-search'], + ownerFeatures: ['git', 'loopx-issue-fix', 'mcp', 'remote-connect', 'remote-ssh', 'remote-ssh-concrete', 'review-platform', 'script-tool-runtime', 'speech', 'workspace-search'], }, { depName: 'base64', @@ -137,7 +137,7 @@ export const optionalDependencyFeatureOwnerRules = [ { depName: 'bitfun-agent-runtime', ownerFeatures: ['deep-research', 'hook-import'] }, { depName: 'bitfun-core-types', ownerFeatures: ['speech'] }, { depName: 'bitfun-product-domains', ownerFeatures: ['canvas-runtime', 'function-agents', 'hook-import', 'miniapp-market', 'miniapp-runtime', 'plugin-source'] }, - { depName: 'bitfun-runtime-ports', ownerFeatures: ['git', 'remote-connect', 'remote-ssh', 'remote-ssh-concrete', 'script-tool-runtime'] }, + { depName: 'bitfun-runtime-ports', ownerFeatures: ['git', 'loopx-issue-fix', 'remote-connect', 'remote-ssh', 'remote-ssh-concrete', 'script-tool-runtime'] }, { depName: 'bitfun-services-core', ownerFeatures: ['browser-control', 'git', 'hook-import', 'mcp', 'miniapp-runtime', 'process-tree', 'remote-connect', 'remote-ssh', 'review-platform', 'workspace-search'], @@ -179,12 +179,12 @@ export const optionalDependencyFeatureOwnerRules = [ { depName: 'ssh_config', ownerFeatures: ['remote-ssh-concrete', 'ssh_config'] }, { depName: 'terminal-core', ownerFeatures: ['remote-ssh', 'remote-ssh-concrete'] }, { depName: 'tar', ownerFeatures: ['speech'] }, - { depName: 'thiserror', ownerFeatures: ['browser-control', 'git', 'hook-import', 'miniapp-market', 'plugin-source', 'remote-ssh', 'remote-ssh-concrete', 'review-platform', 'speech', 'web-tools', 'workspace-search'] }, + { depName: 'thiserror', ownerFeatures: ['browser-control', 'git', 'hook-import', 'loopx-issue-fix', 'miniapp-market', 'plugin-source', 'remote-ssh', 'remote-ssh-concrete', 'review-platform', 'speech', 'web-tools', 'workspace-search'] }, { depName: 'tokio-tungstenite', ownerFeatures: ['remote-connect'] }, { depName: 'tokio-util', ownerFeatures: ['remote-ssh', 'speech'] }, { depName: 'urlencoding', ownerFeatures: ['canvas-runtime', 'miniapp-market', 'remote-connect', 'review-platform'] }, { depName: 'uuid', ownerFeatures: ['canvas-runtime', 'debug-log', 'hook-import', 'miniapp-runtime', 'plugin-source', 'remote-connect', 'remote-ssh-concrete', 'speech'] }, - { depName: 'which', ownerFeatures: ['miniapp-runtime', 'remote-connect', 'script-tool-runtime', 'workspace-search'] }, + { depName: 'which', ownerFeatures: ['loopx-issue-fix', 'miniapp-runtime', 'remote-connect', 'script-tool-runtime', 'workspace-search'] }, { depName: 'windows', ownerFeatures: ['plugin-source', 'review-platform'] }, { depName: 'x25519-dalek', ownerFeatures: ['remote-connect'] }, ], @@ -646,6 +646,7 @@ export const ownerCrateFeatureAssemblyRules = [ 'function-agents', 'git', 'hook-import', + 'loopx-issue-fix', 'miniapp-runtime', 'mcp', 'plugin-source', diff --git a/src/apps/desktop/Cargo.toml b/src/apps/desktop/Cargo.toml index 30caf999b4..fd7cccec55 100644 --- a/src/apps/desktop/Cargo.toml +++ b/src/apps/desktop/Cargo.toml @@ -24,7 +24,7 @@ bitfun-relay-service = { path = "../../crates/services/relay-service" } bitfun-agent-runtime = { path = "../../crates/execution/agent-runtime" } bitfun-runtime-ports = { path = "../../crates/contracts/runtime-ports" } bitfun-product-domains = { path = "../../crates/contracts/product-domains", default-features = false, features = ["appearance-market"] } -bitfun-services-integrations = { path = "../../crates/services/services-integrations", default-features = false, features = ["canvas-runtime", "miniapp-market", "speech"] } +bitfun-services-integrations = { path = "../../crates/services/services-integrations", default-features = false, features = ["canvas-runtime", "loopx-issue-fix", "miniapp-market", "speech"] } bitfun-core-types = { path = "../../crates/contracts/core-types" } bitfun-agent-tools = { path = "../../crates/execution/tool-contracts" } bitfun-transport = { path = "../../crates/adapters/transport", features = ["tauri-adapter"] } diff --git a/src/apps/desktop/src/api/issue_fix_api.rs b/src/apps/desktop/src/api/issue_fix_api.rs new file mode 100644 index 0000000000..34b8490260 --- /dev/null +++ b/src/apps/desktop/src/api/issue_fix_api.rs @@ -0,0 +1,562 @@ +//! Continuous Issue-Fix commands. +//! +//! LoopX Kernel owns the durable issue todos and lifecycle decisions. BitFun's +//! persistent Cron service is only the host wake mechanism for one ordinary +//! Agent session; no BitFun Thread Goal participates in this path. + +use std::path::Path; + +use bitfun_core::service::cron::{ + get_global_cron_service, CreateCronJobRequest, CronJob, CronJobPayload, CronJobRunStatus, + CronJobTarget, CronSchedule, CronWorkspaceRef, UpdateCronJobRequest, +}; +use bitfun_services_integrations::loopx_issue_fix::autonomous::{ + AutonomousControlState, AutonomousIssueFix, AutonomousLightState, IssueSelection, UserDecision, +}; +use bitfun_services_integrations::loopx_issue_fix::LoopxIssueFix; +use log::{error, warn}; +use serde::{Deserialize, Serialize}; +use tauri::State; +use tokio::sync::Mutex; + +use crate::api::app_state::AppState; + +const WAKE_INTERVAL_MS: u64 = 10 * 60 * 1_000; +const JOB_NAME_PREFIX: &str = "LoopX Issue Fix: "; + +/// Serializes host-loop mutations so two concurrent starts cannot both pass +/// the duplicate-job check and create twin cron jobs. +static HOST_LOOP_LOCK: Mutex<()> = Mutex::const_new(()); + +#[derive(Debug, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct IssueFixAvailability { + pub available: bool, + pub program: Option, +} + +#[tauri::command] +pub async fn issue_fix_probe(_state: State<'_, AppState>) -> Result { + match LoopxIssueFix::probe() { + Some(loopx) => Ok(IssueFixAvailability { + available: true, + program: Some(loopx.program().display().to_string()), + }), + None => Ok(IssueFixAvailability { + available: false, + program: None, + }), + } +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct IssueFixAutonomousStatusRequest { + pub repository_path: String, +} + +#[derive(Debug, Clone, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct IssueFixHostLoopState { + pub enabled: bool, + pub job_id: Option, + pub session_id: Option, + pub active_turn_id: Option, + pub next_run_at_ms: Option, + pub last_run_status: Option, + pub last_error: Option, + pub consecutive_failures: u32, +} + +impl Default for IssueFixHostLoopState { + fn default() -> Self { + Self { + enabled: false, + job_id: None, + session_id: None, + active_turn_id: None, + next_run_at_ms: None, + last_run_status: None, + last_error: None, + consecutive_failures: 0, + } + } +} + +#[derive(Debug, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct IssueFixAutonomousStatusResponse { + #[serde(flatten)] + pub control: AutonomousControlState, + pub host_loop: IssueFixHostLoopState, +} + +#[tauri::command] +pub async fn issue_fix_autonomous_status( + _state: State<'_, AppState>, + request: IssueFixAutonomousStatusRequest, +) -> Result { + let repository_path = required_repository_path(&request.repository_path)?; + let loopx = + LoopxIssueFix::probe().ok_or_else(|| "loopx is not installed on this host".to_string())?; + let control = AutonomousIssueFix::new(loopx) + .inspect(repository_path) + .await + .map_err(|error| { + error!("Failed to inspect LoopX Issue-Fix state: {error}"); + format!("Failed to read LoopX Issue-Fix state: {error}") + })?; + let host_loop = host_loop_state(&control.goal_id, repository_path).await; + Ok(IssueFixAutonomousStatusResponse { control, host_loop }) +} + +/// Background-poll response: LoopX todo projection without `quota should-run`. +#[derive(Debug, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct IssueFixAutonomousPollResponse { + #[serde(flatten)] + pub light: AutonomousLightState, + pub host_loop: IssueFixHostLoopState, +} + +/// Cheap status for the panel's poll loop. Unlike `issue_fix_autonomous_status` +/// this never runs `quota should-run` (which appends a LoopX rollout event per +/// call), so polling it on an interval does not grow LoopX's event log. +#[tauri::command] +pub async fn issue_fix_autonomous_poll( + _state: State<'_, AppState>, + request: IssueFixAutonomousStatusRequest, +) -> Result { + let repository_path = required_repository_path(&request.repository_path)?; + let loopx = + LoopxIssueFix::probe().ok_or_else(|| "loopx is not installed on this host".to_string())?; + let light = AutonomousIssueFix::new(loopx) + .poll(repository_path) + .await + .map_err(|error| { + error!("Failed to poll LoopX Issue-Fix todos: {error}"); + format!("Failed to poll LoopX Issue-Fix state: {error}") + })?; + let host_loop = host_loop_state(&light.goal_id, repository_path).await; + Ok(IssueFixAutonomousPollResponse { light, host_loop }) +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct IssueFixAnswerUserQuestionRequest { + pub repository_path: String, + pub todo_id: String, + pub decision: UserDecision, + pub reason: Option, +} + +#[tauri::command] +pub async fn issue_fix_answer_user_question( + _state: State<'_, AppState>, + request: IssueFixAnswerUserQuestionRequest, +) -> Result { + let repository_path = required_repository_path(&request.repository_path)?; + let loopx = + LoopxIssueFix::probe().ok_or_else(|| "loopx is not installed on this host".to_string())?; + let autonomous = AutonomousIssueFix::new(loopx); + let control = autonomous + .answer_user_question( + repository_path, + &request.todo_id, + request.decision, + request.reason.as_deref(), + ) + .await + .map_err(|error| { + error!("Failed to answer LoopX Issue-Fix user question: {error}"); + format!("Failed to answer LoopX Issue-Fix user question: {error}") + })?; + // The wake must not race a concurrent Stop: run_job_now's manual trigger + // bypasses enabled=false, so re-read the job state under the same lock + // Stop holds while disabling. + let _guard = HOST_LOOP_LOCK.lock().await; + let mut host_loop = host_loop_state(&control.goal_id, repository_path).await; + if host_loop.enabled { + if let (Some(cron), Some(job_id)) = (get_global_cron_service(), host_loop.job_id.as_deref()) + { + // The stored heartbeat prompt is a snapshot; a gate answer is a + // natural point to re-sync it with the installed LoopX version. + match autonomous.heartbeat_prompt(repository_path).await { + Ok(prompt) => { + if let Err(error) = cron + .update_job( + job_id, + UpdateCronJobRequest { + payload: Some(CronJobPayload { text: prompt }), + ..UpdateCronJobRequest::default() + }, + ) + .await + { + warn!("Failed to refresh the Issue-Fix heartbeat prompt: {error}"); + } + } + Err(error) => { + warn!("Failed to regenerate the Issue-Fix heartbeat prompt: {error}") + } + } + match cron.run_job_now(job_id).await { + Ok(job) => host_loop = project_host_loop(&job), + Err(error) => warn!( + "LoopX Issue-Fix user decision was recorded but the host loop could not be woken immediately: {error}" + ), + } + } + } + Ok(IssueFixAutonomousStatusResponse { control, host_loop }) +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct IssueFixAutonomousIssueRequest { + pub issue_ref: String, + pub issue_url: String, +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct IssueFixStartAutonomousRequest { + pub session_id: String, + pub repo: String, + pub repository_path: String, + pub issues: Vec, +} + +#[derive(Debug, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct IssueFixStartAutonomousResponse { + #[serde(flatten)] + pub control: AutonomousControlState, + pub host_loop: IssueFixHostLoopState, + pub added_issue_refs: Vec, + pub immediate_turn_id: Option, +} + +#[tauri::command] +pub async fn issue_fix_start_autonomous( + _state: State<'_, AppState>, + request: IssueFixStartAutonomousRequest, +) -> Result { + let repository_path = required_repository_path(&request.repository_path)?; + let session_id = request.session_id.trim(); + if session_id.is_empty() { + return Err("session_id is required for continuous issue fixing".to_string()); + } + let cron = + get_global_cron_service().ok_or_else(|| "Cron service is not initialized".to_string())?; + let loopx = + LoopxIssueFix::probe().ok_or_else(|| "loopx is not installed on this host".to_string())?; + let selections = request + .issues + .into_iter() + .map(|issue| IssueSelection { + issue_ref: issue.issue_ref, + issue_url: issue.issue_url, + }) + .collect::>(); + + let plan = AutonomousIssueFix::new(loopx) + .start(repository_path, request.repo.trim(), &selections) + .await + .map_err(|error| { + error!("Failed to start continuous LoopX Issue-Fix: {error}"); + format!("Failed to start continuous Issue-Fix: {error}") + })?; + + let job_name = job_name(&plan.control.goal_id); + let workspace = CronWorkspaceRef { + workspace_id: None, + workspace_path: repository_path.display().to_string(), + project_workspace_path: Some(repository_path.display().to_string()), + execution_target: None, + remote_connection_id: None, + remote_ssh_host: None, + }; + let target = CronJobTarget::Session { + session_id: session_id.to_string(), + workspace, + }; + let schedule = CronSchedule::Every { + every_ms: WAKE_INTERVAL_MS, + anchor_ms: None, + }; + let payload = CronJobPayload { + text: plan.heartbeat_prompt, + }; + + let _guard = HOST_LOOP_LOCK.lock().await; + let matching = resolve_host_loop_job(&job_name, repository_path).await?; + let job = if let Some(existing) = matching { + cron.update_job( + &existing.id, + UpdateCronJobRequest { + name: Some(job_name), + schedule: Some(schedule), + payload: Some(payload), + enabled: Some(true), + target: Some(target), + }, + ) + .await + } else { + cron.create_job(CreateCronJobRequest { + name: job_name, + schedule, + payload, + enabled: true, + target, + }) + .await + } + .map_err(|error| { + error!("Failed to persist the continuous Issue-Fix host loop: {error}"); + format!("Failed to persist continuous Issue-Fix host loop: {error}") + })?; + + let triggered = cron.run_job_now(&job.id).await.map_err(|error| { + error!("Failed to trigger the continuous Issue-Fix host loop: {error}"); + format!("Failed to trigger continuous Issue-Fix host loop: {error}") + })?; + let immediate_turn_id = triggered.state.active_turn_id.clone(); + let host_loop = project_host_loop(&triggered); + + Ok(IssueFixStartAutonomousResponse { + control: plan.control, + host_loop, + added_issue_refs: plan.added_issue_refs, + immediate_turn_id, + }) +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct IssueFixStopAutonomousRequest { + pub repository_path: String, +} + +/// Disable the host wake loop. LoopX Kernel state (goal, todos, gates) is left +/// untouched: stopping the heartbeat is a host scheduling concern, and a later +/// start resumes exactly where the Kernel says the work stands. +#[tauri::command] +pub async fn issue_fix_stop_autonomous( + _state: State<'_, AppState>, + request: IssueFixStopAutonomousRequest, +) -> Result { + let repository_path = required_repository_path(&request.repository_path)?; + // Stop is the kill switch: it must work even when the LoopX registry is + // broken or the goal identity changed, so a failed lookup only demotes + // which job gets projected, never aborts the disable sweep. + let current_name = match AutonomousIssueFix::identity(repository_path) { + Ok((goal_id, _)) => job_name(&goal_id), + Err(error) => { + warn!("Stopping Issue-Fix host loops without a resolvable LoopX goal: {error}"); + String::new() + } + }; + let cron = + get_global_cron_service().ok_or_else(|| "Cron service is not initialized".to_string())?; + + let _guard = HOST_LOOP_LOCK.lock().await; + // Disable every Issue-Fix loop bound to this repository, not just the + // current goal's: this must also catch jobs orphaned by an older goal + // identity. + let mut stopped: Option = None; + for job in cron.list_jobs().await { + if !job.name.starts_with(JOB_NAME_PREFIX) || !job_targets_repository(&job, repository_path) + { + continue; + } + let disabled = cron + .update_job( + &job.id, + UpdateCronJobRequest { + enabled: Some(false), + ..UpdateCronJobRequest::default() + }, + ) + .await + .map_err(|error| { + error!("Failed to stop the continuous Issue-Fix host loop: {error}"); + format!("Failed to stop continuous Issue-Fix host loop: {error}") + })?; + if disabled.name == current_name || stopped.is_none() { + stopped = Some(disabled); + } + } + Ok(stopped + .as_ref() + .map(project_host_loop) + .unwrap_or_default()) +} + +fn required_repository_path(raw: &str) -> Result<&Path, String> { + let trimmed = raw.trim(); + if trimmed.is_empty() { + return Err("repository_path is required".to_string()); + } + let path = Path::new(trimmed); + if !path.is_dir() { + return Err(format!( + "Repository path does not exist: {}", + path.display() + )); + } + Ok(path) +} + +fn job_name(goal_id: &str) -> String { + format!("{JOB_NAME_PREFIX}{goal_id}") +} + +/// Project the current goal's host loop, tolerating duplicates. +/// +/// Duplicate jobs (from a concurrent start racing the create) must not brick +/// every status call: project the most recently updated one and leave the +/// cleanup to the next start, which holds `HOST_LOOP_LOCK`. +async fn host_loop_state(goal_id: &str, repository_path: &Path) -> IssueFixHostLoopState { + let Some(cron) = get_global_cron_service() else { + return IssueFixHostLoopState::default(); + }; + let mut matching = matching_jobs(&job_name(goal_id), repository_path, cron.list_jobs().await); + if matching.len() > 1 { + warn!( + "Found {} continuous Issue-Fix host loops for goal {goal_id}; projecting the newest", + matching.len() + ); + matching.sort_by_key(|job| std::cmp::Reverse(job.updated_at_ms)); + } + matching + .first() + .map(project_host_loop) + .unwrap_or_default() +} + +/// Pick the canonical host-loop job for `start`, deleting duplicates and +/// disabling stale jobs left behind by an older goal identity. Callers must +/// hold `HOST_LOOP_LOCK`. +async fn resolve_host_loop_job( + name: &str, + repository_path: &Path, +) -> Result, String> { + let Some(cron) = get_global_cron_service() else { + return Err("Cron service is not initialized".to_string()); + }; + let mut canonical: Option = None; + for job in cron.list_jobs().await { + if !job.name.starts_with(JOB_NAME_PREFIX) || !job_targets_repository(&job, repository_path) + { + continue; + } + if job.name != name { + // A job from a previous goal identity (e.g. after re-bootstrap) + // would keep firing its stale prompt invisibly; park it. + if job.enabled { + warn!("Disabling stale continuous Issue-Fix host loop {}", job.name); + let _ = cron + .update_job( + &job.id, + UpdateCronJobRequest { + enabled: Some(false), + ..UpdateCronJobRequest::default() + }, + ) + .await; + } + continue; + } + match &canonical { + Some(kept) if kept.updated_at_ms >= job.updated_at_ms => { + warn!("Deleting duplicate continuous Issue-Fix host loop {}", job.id); + let _ = cron.delete_job(&job.id).await; + } + Some(kept) => { + warn!("Deleting duplicate continuous Issue-Fix host loop {}", kept.id); + let _ = cron.delete_job(&kept.id).await; + canonical = Some(job); + } + None => canonical = Some(job), + } + } + Ok(canonical) +} + +fn matching_jobs(name: &str, repository_path: &Path, jobs: Vec) -> Vec { + jobs.into_iter() + .filter(|job| job.name == name && job_targets_repository(job, repository_path)) + .collect() +} + +fn job_targets_repository(job: &CronJob, repository_path: &Path) -> bool { + same_path(&job.workspace().workspace_path, repository_path) + || job + .workspace() + .project_workspace_path + .as_deref() + .is_some_and(|path| same_path(path, repository_path)) +} + +fn same_path(candidate: &str, expected: &Path) -> bool { + let candidate = candidate.replace('/', "\\"); + let expected = expected.display().to_string().replace('/', "\\"); + let candidate = candidate.trim_end_matches('\\'); + let expected = expected.trim_end_matches('\\'); + // Case-insensitive comparison is a Windows filesystem property; on other + // platforms /repos/Foo and /repos/foo are distinct repositories. + #[cfg(windows)] + { + candidate.eq_ignore_ascii_case(expected) + } + #[cfg(not(windows))] + { + candidate == expected + } +} + +fn run_status_label(status: CronJobRunStatus) -> &'static str { + match status { + CronJobRunStatus::Queued => "queued", + CronJobRunStatus::Running => "running", + CronJobRunStatus::Ok => "ok", + CronJobRunStatus::Error => "error", + CronJobRunStatus::Cancelled => "cancelled", + } +} + +fn project_host_loop(job: &CronJob) -> IssueFixHostLoopState { + IssueFixHostLoopState { + enabled: job.enabled, + job_id: Some(job.id.clone()), + session_id: job.session_id().map(str::to_string), + active_turn_id: job.state.active_turn_id.clone(), + next_run_at_ms: job.state.next_run_at_ms, + last_run_status: job + .state + .last_run_status + .map(|status| run_status_label(status).to_string()), + last_error: job.state.last_error.clone(), + consecutive_failures: job.state.consecutive_failures, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn host_job_name_is_goal_scoped() { + assert_eq!(job_name("bitfun-goal"), "LoopX Issue Fix: bitfun-goal"); + } + + #[test] + fn windows_path_matching_is_case_and_separator_insensitive() { + assert!(same_path( + "C:/codeagent/BitFun/", + Path::new("c:\\codeagent\\bitfun") + )); + } +} diff --git a/src/apps/desktop/src/api/mod.rs b/src/apps/desktop/src/api/mod.rs index f11a5ef845..2635144cd1 100644 --- a/src/apps/desktop/src/api/mod.rs +++ b/src/apps/desktop/src/api/mod.rs @@ -28,6 +28,7 @@ pub mod git_agent_api; pub mod git_api; pub mod i18n_api; pub mod insights_api; +pub mod issue_fix_api; pub mod lsp_api; pub mod lsp_workspace_api; pub mod mcp_api; diff --git a/src/apps/desktop/src/api/remote_workspace_policy.rs b/src/apps/desktop/src/api/remote_workspace_policy.rs index d7df25d145..dd7f140eee 100644 --- a/src/apps/desktop/src/api/remote_workspace_policy.rs +++ b/src/apps/desktop/src/api/remote_workspace_policy.rs @@ -1612,10 +1612,35 @@ pub const REMOTE_WORKSPACE_COMMAND_POLICIES: &[(&str, RemoteWorkspacePolicy)] = "review_platform_get_workspace_snapshot", RemoteWorkspacePolicy::RemoteRouted, ), + ( + "review_platform_list_issues", + RemoteWorkspacePolicy::RemoteRouted, + ), ( "review_platform_update_auth_token", RemoteWorkspacePolicy::WorkspaceAgnostic, ), + ( + "issue_fix_autonomous_status", + RemoteWorkspacePolicy::RemoteUnsupported, + ), + ( + "issue_fix_autonomous_poll", + RemoteWorkspacePolicy::RemoteUnsupported, + ), + ( + "issue_fix_answer_user_question", + RemoteWorkspacePolicy::RemoteUnsupported, + ), + ("issue_fix_probe", RemoteWorkspacePolicy::WorkspaceAgnostic), + ( + "issue_fix_start_autonomous", + RemoteWorkspacePolicy::RemoteUnsupported, + ), + ( + "issue_fix_stop_autonomous", + RemoteWorkspacePolicy::RemoteUnsupported, + ), ("rollback_miniapp", RemoteWorkspacePolicy::LegacyUnaudited), ("rollback_session", RemoteWorkspacePolicy::RemoteUnsupported), ("rollback_to_turn", RemoteWorkspacePolicy::RemoteUnsupported), diff --git a/src/apps/desktop/src/api/review_platform_api.rs b/src/apps/desktop/src/api/review_platform_api.rs index db2803be54..471be92565 100644 --- a/src/apps/desktop/src/api/review_platform_api.rs +++ b/src/apps/desktop/src/api/review_platform_api.rs @@ -3,7 +3,8 @@ use crate::api::app_state::AppState; use bitfun_core::service::review_platform::{ ReviewPlatformCiLog, ReviewPlatformDetailSection, ReviewPlatformError, - ReviewPlatformIssueEvidence, ReviewPlatformKind, ReviewPlatformPullRequestDetail, + ReviewPlatformIssueEvidence, ReviewPlatformIssuePage, ReviewPlatformIssueState, + ReviewPlatformKind, ReviewPlatformListIssuesRequest, ReviewPlatformPullRequestDetail, ReviewPlatformPullRequestDetailPage, ReviewPlatformPullRequestReviewTarget, ReviewPlatformService, ReviewPlatformWorkspaceSnapshot, }; @@ -186,6 +187,35 @@ pub async fn review_platform_get_issue( }) } +/// Enumerate a repository's issues. +/// +/// Returns summary rows only; the caller fetches full evidence per issue when it +/// needs a body and comments. +#[tauri::command] +pub async fn review_platform_list_issues( + _state: State<'_, AppState>, + request: ReviewPlatformListIssuesDto, +) -> Result { + ReviewPlatformService::list_issues(ReviewPlatformListIssuesRequest { + platform: request.platform, + host: &request.host, + project_path: &request.project_path, + state: request.state.unwrap_or_default(), + page: request.page, + per_page: request.per_page, + repository_path: request.repository_path.as_deref(), + }) + .await + .map_err(|error| { + let safe_error = safe_review_platform_error(&error); + error!( + "Failed to list review platform Issues: platform={:?}, host={}, project_path={}, error={}", + request.platform, request.host, request.project_path, safe_error + ); + format!("Failed to list provider Issues: {safe_error}") + }) +} + #[tauri::command] pub async fn review_platform_get_pull_request_review_target_by_identity( _state: State<'_, AppState>, @@ -336,6 +366,21 @@ pub struct ReviewPlatformIssueRequest { pub repository_path: Option, } +/// Owned mirror of `ReviewPlatformListIssuesRequest`, which borrows its strings +/// and so cannot be deserialized directly. +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ReviewPlatformListIssuesDto { + pub platform: ReviewPlatformKind, + pub host: String, + pub project_path: String, + /// Defaults to open issues. + pub state: Option, + pub page: Option, + pub per_page: Option, + pub repository_path: Option, +} + #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct ReviewPlatformPullRequestIdentityRequest { diff --git a/src/apps/desktop/src/lib.rs b/src/apps/desktop/src/lib.rs index 8b8b286afd..1d471450bd 100644 --- a/src/apps/desktop/src/lib.rs +++ b/src/apps/desktop/src/lib.rs @@ -69,6 +69,7 @@ use api::external_sources_api::*; use api::git_agent_api::*; use api::git_api::*; use api::i18n_api::*; +use api::issue_fix_api::*; use api::lsp_api::*; use api::lsp_workspace_api::*; use api::mcp_api::*; @@ -1332,6 +1333,13 @@ pub async fn run() { review_platform_get_pull_request_detail, review_platform_get_pull_request_review_target, review_platform_get_issue, + review_platform_list_issues, + issue_fix_probe, + issue_fix_autonomous_status, + issue_fix_autonomous_poll, + issue_fix_answer_user_question, + issue_fix_start_autonomous, + issue_fix_stop_autonomous, review_platform_get_pull_request_review_target_by_identity, review_platform_get_pull_request_detail_page, review_platform_get_pull_request_ci_log, diff --git a/src/crates/assembly/core/src/miniapp/builtin/mod.rs b/src/crates/assembly/core/src/miniapp/builtin/mod.rs index 4696ab466e..fe40092ffb 100644 --- a/src/crates/assembly/core/src/miniapp/builtin/mod.rs +++ b/src/crates/assembly/core/src/miniapp/builtin/mod.rs @@ -23,7 +23,7 @@ use chrono::Utc; use std::path::Path; use std::sync::Arc; -const RETIRED_BUILTIN_APP_IDS: &[&str] = &["builtin-pr-review"]; +const RETIRED_BUILTIN_APP_IDS: &[&str] = &["builtin-pr-review", "builtin-loopx-console"]; /// Seed all built-in MiniApps into the user data directory. Idempotent: skips apps /// whose on-disk marker hash matches the bundled content. User's `storage.json` @@ -460,16 +460,19 @@ mod tests { } #[tokio::test] - async fn builtin_seed_retires_the_removed_pr_review_bundle_only_when_marked_builtin() { + async fn builtin_seed_retires_removed_bundles_only_when_marked_builtin() { let manager = test_manager(); - let app_dir = manager.path_manager().miniapp_dir("builtin-pr-review"); - tokio::fs::create_dir_all(&app_dir).await.unwrap(); - write_outdated_builtin_marker(&app_dir).await; + for app_id in RETIRED_BUILTIN_APP_IDS { + let app_dir = manager.path_manager().miniapp_dir(app_id); + tokio::fs::create_dir_all(&app_dir).await.unwrap(); + write_outdated_builtin_marker(&app_dir).await; - seed_builtin_miniapps(&manager).await.unwrap(); + seed_builtin_miniapps(&manager).await.unwrap(); - assert!(!app_dir.exists()); + assert!(!app_dir.exists()); + } + let app_dir = manager.path_manager().miniapp_dir("builtin-loopx-console"); tokio::fs::create_dir_all(&app_dir).await.unwrap(); tokio::fs::write(app_dir.join("meta.json"), "{}") .await diff --git a/src/crates/assembly/core/src/service/review_platform/mod.rs b/src/crates/assembly/core/src/service/review_platform/mod.rs index 7ad3327a52..f79b48f7ca 100644 --- a/src/crates/assembly/core/src/service/review_platform/mod.rs +++ b/src/crates/assembly/core/src/service/review_platform/mod.rs @@ -15,9 +15,10 @@ pub use bitfun_services_integrations::review_platform::{ ReviewPlatformCapabilities, ReviewPlatformCiItem, ReviewPlatformCiLog, ReviewPlatformCommit, ReviewPlatformCreatePullRequestRequest, ReviewPlatformDetailSection, ReviewPlatformError, ReviewPlatformFile, ReviewPlatformIssueComment, ReviewPlatformIssueEvidence, - ReviewPlatformKind, ReviewPlatformPullRequest, ReviewPlatformPullRequestDetail, - ReviewPlatformPullRequestDetailPage, ReviewPlatformPullRequestFileDiff, - ReviewPlatformPullRequestReviewTarget, ReviewPlatformRemote, + ReviewPlatformIssuePage, ReviewPlatformIssueState, ReviewPlatformIssueSummary, + ReviewPlatformKind, ReviewPlatformListIssuesRequest, ReviewPlatformPullRequest, + ReviewPlatformPullRequestDetail, ReviewPlatformPullRequestDetailPage, + ReviewPlatformPullRequestFileDiff, ReviewPlatformPullRequestReviewTarget, ReviewPlatformRemote, ReviewPlatformReplyToThreadRequest, ReviewPlatformRepositoryRef, ReviewPlatformRequestChangesRequest, ReviewPlatformResolveThreadRequest, ReviewPlatformSubmitReviewRequest, ReviewPlatformThread, ReviewPlatformThreadKind, @@ -173,6 +174,12 @@ impl ReviewPlatformService { .await } + pub async fn list_issues( + request: ReviewPlatformListIssuesRequest<'_>, + ) -> Result { + owner_service()?.list_issues(request).await + } + pub async fn pull_request_review_target_by_identity( platform: ReviewPlatformKind, host: &str, diff --git a/src/crates/assembly/core/src/service/worktree/mod.rs b/src/crates/assembly/core/src/service/worktree/mod.rs index d7e153b31e..51021854c6 100644 --- a/src/crates/assembly/core/src/service/worktree/mod.rs +++ b/src/crates/assembly/core/src/service/worktree/mod.rs @@ -1863,7 +1863,21 @@ fn path_is_within_root(path: &Path, root: &Path) -> bool { } fn path_string(path: &Path) -> String { - path.to_string_lossy().replace('\\', "/") + display_path_string(path.to_string_lossy().as_ref()) +} + +fn display_path_string(value: &str) -> String { + let normalized = value.replace('\\', "/"); + #[cfg(windows)] + { + if let Some(rest) = normalized.strip_prefix("//?/UNC/") { + return format!("//{rest}"); + } + if let Some(rest) = normalized.strip_prefix("//?/") { + return rest.to_string(); + } + } + normalized } fn current_unix_ms() -> u64 { @@ -1995,6 +2009,8 @@ fn map_git_error(git_error: GitError) -> WorktreeError { #[cfg(test)] mod tests { + #[cfg(windows)] + use super::display_path_string; use super::{ automatic_delete_candidate_ids, managed_target_path, managed_worktree_directory_name, path_is_within_root, repository_id, resolve_managed_root, sanitize_worktree_project_label, @@ -2039,6 +2055,19 @@ mod tests { ); } + #[cfg(windows)] + #[test] + fn path_string_strips_extended_windows_prefix_for_ui_contracts() { + assert_eq!( + display_path_string(r"\\?\C:\Users\huawei\.bitfun\worktrees\repo"), + "C:/Users/huawei/.bitfun/worktrees/repo" + ); + assert_eq!( + display_path_string(r"\\?\UNC\server\share\repo"), + "//server/share/repo" + ); + } + #[test] fn managed_directory_name_includes_project_name_and_short_worktree_id() { let project = Path::new("projects").join("BitFun"); diff --git a/src/crates/execution/tool-execution/src/fs/edit_file.rs b/src/crates/execution/tool-execution/src/fs/edit_file.rs index 0a3bc5a5dd..fbce1b90f5 100644 --- a/src/crates/execution/tool-execution/src/fs/edit_file.rs +++ b/src/crates/execution/tool-execution/src/fs/edit_file.rs @@ -141,25 +141,16 @@ fn find_actual_string(file_content: &str, search_string: &str) -> Option return None; } - let normalized_search_chars: Vec = search_chars - .iter() - .copied() - .map(normalize_quote_char) - .collect(); - - for start in 0..=file_chars.len() - search_chars.len() { - let window_matches = file_chars[start..start + search_chars.len()] - .iter() - .copied() - .map(normalize_quote_char) - .eq(normalized_search_chars.iter().copied()); - if window_matches { - return Some( - file_chars[start..start + search_chars.len()] - .iter() - .collect(), - ); - } + // Quote-normalized matching: build normalized versions of both strings + // and use str::find() (O(n) Two-Way) instead of a character-by-character + // scan (O(n*m)) to locate the match position (issue #1650). + let normalized_file_str: String = file_chars.iter().copied().map(normalize_quote_char).collect(); + let normalized_search_str: String = search_chars.iter().copied().map(normalize_quote_char).collect(); + + if let Some(byte_pos) = normalized_file_str.find(&normalized_search_str) { + let char_start = normalized_file_str[..byte_pos].chars().count(); + let char_end = char_start + search_chars.len(); + return Some(file_chars[char_start..char_end].iter().collect()); } None @@ -238,8 +229,6 @@ fn edit_string_candidates( let tabs_to_spaces_new = convert_tabs_to_spaces(new_string, tab_width); push_candidate(tabs_to_spaces_old.clone(), tabs_to_spaces_new.clone()); - // Also try quote-normalized variant (e.g. curly quotes in file - // after whitespace normalization). if let Some(actual_old) = find_actual_string(content, &tabs_to_spaces_old) { push_candidate(actual_old, tabs_to_spaces_new); } diff --git a/src/crates/services/services-integrations/Cargo.toml b/src/crates/services/services-integrations/Cargo.toml index 9b07f24c05..ec60e23501 100644 --- a/src/crates/services/services-integrations/Cargo.toml +++ b/src/crates/services/services-integrations/Cargo.toml @@ -125,6 +125,17 @@ git = [ "tokio/time", ] file-watch = ["notify", "tokio/rt", "tokio/sync"] +# Automatic repository issue fixing driven by the external `loopx` CLI. +# Included in `product-full`. +loopx-issue-fix = [ + "async-trait", + "bitfun-runtime-ports", + "review-platform", + "thiserror", + "tokio/macros", + "tokio/process", + "which", +] function-agents = [ "bitfun-product-domains/function-agents", "dep:bitfun-product-domains", @@ -374,6 +385,7 @@ product-full = [ "function-agents", "git", "hook-import", + "loopx-issue-fix", "miniapp-runtime", "mcp", "plugin-source", @@ -415,6 +427,10 @@ required-features = ["function-agents"] name = "git_contracts" required-features = ["git"] +[[test]] +name = "loopx_issue_fix_contracts" +required-features = ["loopx-issue-fix"] + [[test]] name = "mcp_contracts" required-features = ["mcp"] diff --git a/src/crates/services/services-integrations/src/git/managed_worktree.rs b/src/crates/services/services-integrations/src/git/managed_worktree.rs index fbe33b1df7..b4ab4f977c 100644 --- a/src/crates/services/services-integrations/src/git/managed_worktree.rs +++ b/src/crates/services/services-integrations/src/git/managed_worktree.rs @@ -10,7 +10,21 @@ use tokio::io::AsyncWriteExt; use tokio::task; fn normalized_path(path: &Path) -> String { - path.to_string_lossy().replace('\\', "/") + normalize_git_cli_path(path.to_string_lossy().as_ref()) +} + +fn normalize_git_cli_path(value: &str) -> String { + let normalized = value.replace('\\', "/"); + #[cfg(windows)] + { + if let Some(rest) = normalized.strip_prefix("//?/UNC/") { + return format!("//{rest}"); + } + if let Some(rest) = normalized.strip_prefix("//?/") { + return rest.to_string(); + } + } + normalized } fn parse_nul_paths(bytes: &[u8]) -> Result, GitError> { @@ -476,7 +490,7 @@ impl GitService { #[cfg(test)] mod tests { - use super::GitService; + use super::{normalize_git_cli_path, GitService}; use std::fs; use std::path::Path; use std::process::Command; @@ -514,6 +528,19 @@ mod tests { (temp, repository) } + #[cfg(windows)] + #[test] + fn normalized_path_strips_extended_windows_prefix_for_git_cli() { + assert_eq!( + normalize_git_cli_path(r"\\?\C:\Users\huawei\.bitfun\worktrees\repo"), + "C:/Users/huawei/.bitfun/worktrees/repo" + ); + assert_eq!( + normalize_git_cli_path(r"\\?\UNC\server\share\repo"), + "//server/share/repo" + ); + } + #[tokio::test] async fn detached_worktrees_from_the_same_commit_are_independent() { let (temp, repository) = initialized_repository(); diff --git a/src/crates/services/services-integrations/src/lib.rs b/src/crates/services/services-integrations/src/lib.rs index 81b53a6afc..3d8e59ddb7 100644 --- a/src/crates/services/services-integrations/src/lib.rs +++ b/src/crates/services/services-integrations/src/lib.rs @@ -33,6 +33,9 @@ pub mod git; #[cfg(feature = "hook-import")] pub mod hook_import; +#[cfg(feature = "loopx-issue-fix")] +pub mod loopx_issue_fix; + #[cfg(feature = "mcp")] pub mod mcp; diff --git a/src/crates/services/services-integrations/src/loopx_issue_fix/autonomous.rs b/src/crates/services/services-integrations/src/loopx_issue_fix/autonomous.rs new file mode 100644 index 0000000000..fe3ff343d2 --- /dev/null +++ b/src/crates/services/services-integrations/src/loopx_issue_fix/autonomous.rs @@ -0,0 +1,995 @@ +//! LoopX-owned control state for continuous issue fixing. +//! +//! BitFun persists no issue queue here. Selected issues are written directly as +//! LoopX agent todos, and every UI refresh is rebuilt from LoopX todo/quota +//! packets. The host scheduler only wakes the generated heartbeat prompt. + +use std::collections::HashSet; +use std::path::{Path, PathBuf}; + +use serde::{Deserialize, Serialize}; +use serde_json::Value; +use thiserror::Error; + +use super::{LoopxIssueFix, LoopxIssueFixError}; + +const ISSUE_INTAKE_ACTION: &str = "issue_fix_intake"; +const MAX_USER_REASON_CHARS: usize = 240; + +/// Host preamble prepended to LoopX's generated heartbeat contract. +/// +/// LoopX owns every lifecycle rule (the `--compact` task body below it); this +/// header only translates host-surface concerns — where the agent runs, how +/// BitFun projects user gates, and what a tick means here. It must never add +/// lifecycle branching of its own. +const HEARTBEAT_HOST_PREAMBLE: &str = "\ +You are BitFun's continuous Issue-Fix host agent. Each scheduled message in this \ +conversation is one LoopX heartbeat tick. LoopX Kernel state is the ONLY source of \ +truth: on every tick re-read it fresh through the `loopx` CLI, and disregard any \ +conclusion from earlier messages in this conversation that conflicts with the \ +current packets. Never invent issue or PR state, and never record progress \ +anywhere except through LoopX writebacks. + +The goal is continuous multi-issue repair. Each selected issue is an independent \ +one-off advancement todo that must travel the full lifecycle — reproduce, patch in \ +an isolated worktree, validate (failure-before / pass-after), publish, then monitor \ +its pull request through the grouped lifecycle monitors — to validated terminal \ +closeout, with every transition written back to LoopX. Do one bounded, verifiable \ +segment per tick, then stop and wait for the next tick. + +Host surface notes: +- You run inside a BitFun desktop chat session; BitFun's host loop owns scheduling. \ +LoopX skill packs are not installed here — follow the contract below directly and \ +consult `loopx --help` when unsure. +- Raise human decisions ONLY as typed LoopX user todos: `loopx todo add --role user \ +--task-class user_gate --unblocks-todo-id ...`, always \ +linking the gate to the blocked todo. BitFun's Issue-Fix panel projects open gates \ +to the user and records each decision through the LoopX todo lifecycle; a plain \ +chat reply never grants authority. +- NOTIFY / DONT_NOTIFY in the contract below control only the final chat summary: \ +for NOTIFY end with a concise user-facing summary (in the contract's notification \ +language), for DONT_NOTIFY end with a single quiet status line. + +--- LoopX heartbeat contract follows ---"; +const REQUIRED_CAPABILITIES: [&str; 4] = ["shell", "filesystem_write", "git", "network"]; +const HEARTBEAT_CAPABILITIES: [&str; 5] = [ + "shell", + "filesystem_write", + "git", + "network", + "external_evidence_poll", +]; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct IssueSelection { + pub issue_ref: String, + pub issue_url: String, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct AutonomousIssueTodo { + pub issue_ref: String, + pub issue_url: String, + pub todo_id: String, + pub status: String, + pub selected: bool, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct AutonomousUserQuestion { + pub todo_id: String, + pub prompt: String, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum UserDecision { + Approve, + Reject, + Cancel, +} + +impl UserDecision { + fn as_loopx_value(self) -> &'static str { + match self { + Self::Approve => "approve", + Self::Reject => "reject", + Self::Cancel => "cancel", + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct AutonomousControlState { + pub goal_id: String, + pub agent_id: String, + pub kernel_state: String, + pub should_run: bool, + pub action_required: bool, + pub recommended_action: Option, + pub gate_prompt: Option, + pub selected_todo_id: Option, + pub issues: Vec, + pub user_question: Option, +} + +/// A cheap projection for background polling: todo list only, no `quota +/// should-run`. LoopX appends a rollout event on every `should-run` call, so a +/// UI poll loop must not run it; gates and issue todos are fully derivable +/// from `todo list`, which is read-only. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct AutonomousLightState { + pub goal_id: String, + pub agent_id: String, + pub action_required: bool, + pub issues: Vec, + pub user_question: Option, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AutonomousStartPlan { + pub control: AutonomousControlState, + pub heartbeat_prompt: String, + pub added_issue_refs: Vec, +} + +#[derive(Debug, Error)] +pub enum AutonomousIssueFixError { + #[error(transparent)] + Loopx(#[from] LoopxIssueFixError), + #[error("failed to read LoopX registry {path}: {source}")] + RegistryRead { + path: PathBuf, + #[source] + source: std::io::Error, + }, + #[error("LoopX registry {path} is not valid JSON: {source}")] + RegistryJson { + path: PathBuf, + #[source] + source: serde_json::Error, + }, + #[error("expected exactly one active LoopX goal, found {0}")] + ActiveGoalCount(usize), + #[error("expected exactly one registered LoopX agent for goal {goal_id}, found {count}")] + RegisteredAgentCount { goal_id: String, count: usize }, + #[error("LoopX {command} packet is missing required field {field}")] + MissingField { + command: &'static str, + field: &'static str, + }, + #[error("invalid issue selection: {0}")] + InvalidSelection(String), + #[error("invalid user response: {0}")] + InvalidUserResponse(String), +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct ControlIdentity { + goal_id: String, + agent_id: String, +} + +#[derive(Debug, Clone)] +pub struct AutonomousIssueFix { + loopx: LoopxIssueFix, +} + +impl AutonomousIssueFix { + pub fn new(loopx: LoopxIssueFix) -> Self { + Self { loopx } + } + + /// Project the current Issue-Fix surface from LoopX without writing state. + pub async fn inspect( + &self, + repository_path: &Path, + ) -> Result { + let identity = read_identity(repository_path)?; + self.inspect_with_identity(repository_path, &identity).await + } + + /// Cheap read-only projection for background polling: `todo list` only. + /// + /// Unlike [`Self::inspect`], this never invokes `quota should-run`, which + /// appends a rollout event per call and would grow LoopX's event log + /// unboundedly under a poll loop. + pub async fn poll( + &self, + repository_path: &Path, + ) -> Result { + let identity = read_identity(repository_path)?; + let todos = self.list_todos(repository_path, &identity).await?; + let issues = todos + .iter() + .filter_map(project_issue_todo) + .collect::>(); + let user_question = project_user_question(&todos, &issues); + Ok(AutonomousLightState { + goal_id: identity.goal_id, + agent_id: identity.agent_id, + action_required: user_question.is_some(), + issues, + user_question, + }) + } + + /// Resolve the control identity (active goal, registered agent) from the + /// project registry without touching LoopX state. Host-loop management + /// needs the goal id even when no LoopX command should run. + pub fn identity( + repository_path: &Path, + ) -> Result<(String, String), AutonomousIssueFixError> { + read_identity(repository_path).map(|identity| (identity.goal_id, identity.agent_id)) + } + + /// Regenerate the full host heartbeat prompt for the registered goal. + /// + /// The stored cron payload is a snapshot; callers should refresh it at + /// every natural write point (start, gate answers) so LoopX upgrades that + /// change the generated contract propagate without a manual restart. + pub async fn heartbeat_prompt( + &self, + repository_path: &Path, + ) -> Result { + let identity = read_identity(repository_path)?; + self.heartbeat_prompt_with_identity(repository_path, &identity) + .await + } + + /// Persist every selected issue as a LoopX todo, then generate the host + /// heartbeat from the resulting Kernel state. + pub async fn start( + &self, + repository_path: &Path, + repo: &str, + issues: &[IssueSelection], + ) -> Result { + if issues.is_empty() { + return Err(AutonomousIssueFixError::InvalidSelection( + "at least one issue is required".to_string(), + )); + } + let identity = read_identity(repository_path)?; + let mut existing = self + .list_issue_todos(repository_path, &identity) + .await? + .into_iter() + .map(|todo| todo.issue_ref) + .collect::>(); + let mut added_issue_refs = Vec::new(); + + for issue in issues { + validate_selection(issue)?; + if !existing.insert(issue.issue_ref.clone()) { + continue; + } + let task_repository = task_repository(repo, &issue.issue_url)?; + let text = issue_todo_text(repo, issue); + let mut args = vec![ + "todo".to_string(), + "add".to_string(), + "--goal-id".to_string(), + identity.goal_id.clone(), + "--role".to_string(), + "agent".to_string(), + "--text".to_string(), + text, + "--task-class".to_string(), + "advancement_task".to_string(), + "--action-kind".to_string(), + ISSUE_INTAKE_ACTION.to_string(), + "--task-repository".to_string(), + task_repository, + "--claimed-by".to_string(), + identity.agent_id.clone(), + ]; + for capability in REQUIRED_CAPABILITIES { + args.push("--required-capability".to_string()); + args.push(capability.to_string()); + } + self.loopx.json_in(repository_path, args).await?; + added_issue_refs.push(issue.issue_ref.clone()); + } + + let (control, heartbeat_prompt) = tokio::try_join!( + self.inspect_with_identity(repository_path, &identity), + self.heartbeat_prompt_with_identity(repository_path, &identity), + )?; + + Ok(AutonomousStartPlan { + control, + heartbeat_prompt, + added_issue_refs, + }) + } + + /// Resolve the currently projected Issue-Fix user gate through LoopX's + /// typed todo lifecycle. Only the projected `user_gate` todo is accepted; + /// other user todos (reading queues, `user_action`) are rejected. + pub async fn answer_user_question( + &self, + repository_path: &Path, + todo_id: &str, + decision: UserDecision, + reason: Option<&str>, + ) -> Result { + let identity = read_identity(repository_path)?; + let current = self + .inspect_with_identity(repository_path, &identity) + .await?; + let question = current.user_question.as_ref().ok_or_else(|| { + AutonomousIssueFixError::InvalidUserResponse( + "there is no open Issue-Fix user question".to_string(), + ) + })?; + if question.todo_id != todo_id.trim() { + return Err(AutonomousIssueFixError::InvalidUserResponse(format!( + "todo {} is not the current Issue-Fix user question", + todo_id.trim() + ))); + } + + let args = user_decision_args(&identity, &question.todo_id, decision, reason)?; + self.loopx.json_in(repository_path, args).await?; + self.inspect_with_identity(repository_path, &identity).await + } + + async fn inspect_with_identity( + &self, + repository_path: &Path, + identity: &ControlIdentity, + ) -> Result { + let quota_args = scheduler_args("quota", "should-run", identity, None); + let (quota, todos) = tokio::try_join!( + async { + self.loopx + .json_in(repository_path, quota_args) + .await + .map_err(AutonomousIssueFixError::from) + }, + self.list_todos(repository_path, identity), + )?; + + let mut issues = todos + .iter() + .filter_map(project_issue_todo) + .collect::>(); + let selected_todo_id = optional_string("a, "selected_todo", "todo_id") + .filter(|todo_id| issues.iter().any(|issue| issue.todo_id == *todo_id)); + for issue in &mut issues { + issue.selected = selected_todo_id.as_deref() == Some(issue.todo_id.as_str()); + } + let user_question = project_user_question(&todos, &issues); + let action_required = user_question.is_some(); + + Ok(AutonomousControlState { + goal_id: identity.goal_id.clone(), + agent_id: identity.agent_id.clone(), + kernel_state: required_string("a, "quota should-run", "state")?, + should_run: quota + .get("should_run") + .and_then(Value::as_bool) + .unwrap_or(false), + action_required, + recommended_action: optional_top_string("a, "recommended_action"), + gate_prompt: user_question + .as_ref() + .map(|question| question.prompt.clone()), + selected_todo_id, + issues, + user_question, + }) + } + + async fn list_issue_todos( + &self, + repository_path: &Path, + identity: &ControlIdentity, + ) -> Result, AutonomousIssueFixError> { + let todos = self.list_todos(repository_path, identity).await?; + Ok(todos.iter().filter_map(project_issue_todo).collect()) + } + + async fn list_todos( + &self, + repository_path: &Path, + identity: &ControlIdentity, + ) -> Result, AutonomousIssueFixError> { + let packet = self + .loopx + .json_in( + repository_path, + [ + "todo", + "list", + "--goal-id", + identity.goal_id.as_str(), + "--agent-id", + identity.agent_id.as_str(), + ], + ) + .await?; + packet + .get("todos") + .and_then(Value::as_array) + .cloned() + .ok_or(AutonomousIssueFixError::MissingField { + command: "todo list", + field: "todos", + }) + } + + async fn heartbeat_prompt_with_identity( + &self, + repository_path: &Path, + identity: &ControlIdentity, + ) -> Result { + // `--compact` rather than `--thin`: the thin dispatcher delegates to + // LoopX skill packs (`loopx-project`) that are not installed in a + // BitFun agent session, while the compact body carries the full + // should_run lifecycle inline. + let packet = self + .loopx + .json_in( + repository_path, + scheduler_args("heartbeat-prompt", "", identity, Some("--compact")), + ) + .await?; + let task_body = required_string(&packet, "heartbeat-prompt", "task_body")?; + Ok(compose_heartbeat_prompt(&task_body)) + } +} + +/// Wrap LoopX's generated contract with the BitFun host preamble. +fn compose_heartbeat_prompt(task_body: &str) -> String { + format!("{HEARTBEAT_HOST_PREAMBLE}\n\n{task_body}") +} + +fn scheduler_args( + command: &str, + subcommand: &str, + identity: &ControlIdentity, + prompt_mode: Option<&str>, +) -> Vec { + let mut args = vec![command.to_string()]; + if !subcommand.is_empty() { + args.push(subcommand.to_string()); + } + args.extend([ + "--goal-id".to_string(), + identity.goal_id.clone(), + "--agent-id".to_string(), + identity.agent_id.clone(), + "--host-surface".to_string(), + "local_scheduler".to_string(), + "--scheduler-owner".to_string(), + "host_automation".to_string(), + "--execution-mode".to_string(), + "hosted_automation".to_string(), + ]); + for capability in HEARTBEAT_CAPABILITIES { + args.push("--available-capability".to_string()); + args.push(capability.to_string()); + } + if let Some(mode) = prompt_mode { + args.push(mode.to_string()); + } + args +} + +fn read_identity(repository_path: &Path) -> Result { + let path = repository_path.join(".loopx").join("registry.json"); + let bytes = std::fs::read(&path).map_err(|source| AutonomousIssueFixError::RegistryRead { + path: path.clone(), + source, + })?; + let registry: Value = + serde_json::from_slice(&bytes).map_err(|source| AutonomousIssueFixError::RegistryJson { + path: path.clone(), + source, + })?; + let active_goals = registry + .get("goals") + .and_then(Value::as_array) + .into_iter() + .flatten() + .filter(|goal| goal.get("status").and_then(Value::as_str) == Some("active")) + .collect::>(); + if active_goals.len() != 1 { + return Err(AutonomousIssueFixError::ActiveGoalCount(active_goals.len())); + } + let goal = active_goals[0]; + let goal_id = goal + .get("id") + .and_then(Value::as_str) + .filter(|value| !value.trim().is_empty()) + .ok_or(AutonomousIssueFixError::MissingField { + command: "registry", + field: "goals[].id", + })? + .to_string(); + let agents = goal + .pointer("/coordination/registered_agents") + .and_then(Value::as_array) + .into_iter() + .flatten() + .filter_map(Value::as_str) + .filter(|value| !value.trim().is_empty()) + .collect::>(); + if agents.len() != 1 { + return Err(AutonomousIssueFixError::RegisteredAgentCount { + goal_id, + count: agents.len(), + }); + } + Ok(ControlIdentity { + goal_id, + agent_id: agents[0].to_string(), + }) +} + +fn project_issue_todo(todo: &Value) -> Option { + if todo.get("role").and_then(Value::as_str) != Some("agent") { + return None; + } + let action_kind = todo.get("action_kind").and_then(Value::as_str)?; + if action_kind != ISSUE_INTAKE_ACTION { + return None; + } + let text = todo.get("text").and_then(Value::as_str)?; + let (issue_url, issue_ref) = explicit_issue_url(text)?; + Some(AutonomousIssueTodo { + issue_ref, + issue_url, + todo_id: todo.get("todo_id")?.as_str()?.to_string(), + status: todo + .get("status") + .and_then(Value::as_str) + .unwrap_or("open") + .to_string(), + selected: false, + }) +} + +/// Project the first open Issue-Fix user gate straight from the todo list. +/// +/// `quota should-run` also previews gates, but its `gate_open_items` lane is +/// compacted to two entries; enumerating the todo list is the only complete +/// source, and it keeps gate projection available to the quota-free poll path. +/// +/// Issue-linked gates (unblocking a managed intake todo) win, but an open gate +/// must never be invisible — the Kernel blocks on it either way — so unlinked +/// or goal-wide gates are surfaced as a fallback. Only `user_gate` todos ever +/// project; other user todos (reading queues, `user_action`) stay out. +fn project_user_question( + todos: &[Value], + issues: &[AutonomousIssueTodo], +) -> Option { + let open_gates = todos + .iter() + .filter(|todo| { + todo.get("role").and_then(Value::as_str) == Some("user") + && todo.get("status").and_then(Value::as_str) == Some("open") + && todo.get("task_class").and_then(Value::as_str) == Some("user_gate") + }) + .collect::>(); + let linked = open_gates.iter().find(|todo| { + todo.get("unblocks_todo_id") + .and_then(Value::as_str) + .is_some_and(|unblocks| issues.iter().any(|issue| issue.todo_id == unblocks)) + }); + let todo = linked.or(open_gates.first())?; + Some(AutonomousUserQuestion { + todo_id: todo.get("todo_id")?.as_str()?.to_string(), + prompt: todo.get("text")?.as_str()?.to_string(), + }) +} + +fn user_decision_args( + identity: &ControlIdentity, + todo_id: &str, + decision: UserDecision, + reason: Option<&str>, +) -> Result, AutonomousIssueFixError> { + let reason = reason.map(str::trim).filter(|value| !value.is_empty()); + if reason.is_some_and(|value| value.chars().count() > MAX_USER_REASON_CHARS) { + return Err(AutonomousIssueFixError::InvalidUserResponse(format!( + "reason must be at most {MAX_USER_REASON_CHARS} characters" + ))); + } + let note = reason + .map(str::to_string) + .unwrap_or_else(|| "Submitted from the BitFun continuous Issue-Fix panel.".to_string()); + Ok(vec![ + "todo".to_string(), + "complete".to_string(), + "--goal-id".to_string(), + identity.goal_id.clone(), + "--role".to_string(), + "user".to_string(), + "--todo-id".to_string(), + todo_id.to_string(), + "--agent-id".to_string(), + identity.agent_id.clone(), + "--decision-outcome".to_string(), + decision.as_loopx_value().to_string(), + "--note".to_string(), + note, + ]) +} + +fn explicit_issue_url(text: &str) -> Option<(String, String)> { + for token in text.split(|character: char| character.is_whitespace() || character == '(') { + let url = token.trim_matches(|character: char| { + matches!(character, ')' | ']' | ',' | '.' | ';' | ':' | '`') + }); + let marker = "/issues/"; + let Some(marker_index) = url.find(marker) else { + continue; + }; + if !(url.starts_with("https://") || url.starts_with("http://")) { + continue; + } + let suffix = &url[marker_index + marker.len()..]; + let issue_ref = suffix + .split(|character: char| !character.is_ascii_alphanumeric() && character != '-') + .next() + .filter(|value| !value.is_empty())?; + return Some((url.to_string(), issue_ref.to_string())); + } + None +} + +fn validate_selection(issue: &IssueSelection) -> Result<(), AutonomousIssueFixError> { + let issue_ref = issue.issue_ref.trim(); + if issue_ref.is_empty() + || issue_ref + .chars() + .any(|character| !character.is_ascii_alphanumeric() && character != '-') + { + return Err(AutonomousIssueFixError::InvalidSelection(format!( + "unsupported issue ref {:?}", + issue.issue_ref + ))); + } + let Some((_, url_ref)) = explicit_issue_url(&issue.issue_url) else { + return Err(AutonomousIssueFixError::InvalidSelection(format!( + "issue URL must be an explicit http(s) /issues/ URL: {}", + issue.issue_url + ))); + }; + if url_ref != issue_ref { + return Err(AutonomousIssueFixError::InvalidSelection(format!( + "issue ref {} does not match URL ref {}", + issue_ref, url_ref + ))); + } + Ok(()) +} + +fn task_repository(repo: &str, issue_url: &str) -> Result { + let host = issue_url + .split_once("://") + .and_then(|(_, rest)| rest.split('/').next()) + .filter(|value| !value.is_empty()) + .ok_or_else(|| { + AutonomousIssueFixError::InvalidSelection(format!( + "cannot determine provider host from {issue_url}" + )) + })?; + let repo = repo.trim().trim_matches('/'); + if repo.split('/').count() < 2 { + return Err(AutonomousIssueFixError::InvalidSelection(format!( + "repository identity must be owner/repo: {repo}" + ))); + } + Ok(format!("git:{host}/{repo}")) +} + +fn issue_todo_text(repo: &str, issue: &IssueSelection) -> String { + format!( + "[P0] Advance issue-fix for {repo}#{} ({}): run the canonical LoopX Issue-Fix lifecycle through validated terminal closeout and write every transition back to LoopX.", + issue.issue_ref, issue.issue_url + ) +} + +fn required_string( + value: &Value, + command: &'static str, + field: &'static str, +) -> Result { + value + .get(field) + .and_then(Value::as_str) + .filter(|value| !value.trim().is_empty()) + .map(str::to_string) + .ok_or(AutonomousIssueFixError::MissingField { command, field }) +} + +fn optional_top_string(value: &Value, field: &str) -> Option { + value + .get(field) + .and_then(Value::as_str) + .filter(|value| !value.trim().is_empty()) + .map(str::to_string) +} + +fn optional_string(value: &Value, object: &str, field: &str) -> Option { + value + .get(object) + .and_then(|object| object.get(field)) + .and_then(Value::as_str) + .filter(|value| !value.trim().is_empty()) + .map(str::to_string) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn projects_only_explicit_issue_fix_todos() { + let todo = serde_json::json!({ + "role": "agent", + "action_kind": "issue_fix_intake", + "text": "[P0] Advance issue-fix for owner/repo#1849 (https://github.com/owner/repo/issues/1849): continue.", + "todo_id": "todo_1849", + "status": "open" + }); + + let projected = project_issue_todo(&todo).expect("issue todo projects"); + assert_eq!(projected.issue_ref, "1849"); + assert_eq!( + projected.issue_url, + "https://github.com/owner/repo/issues/1849" + ); + assert_eq!(projected.status, "open"); + } + + #[test] + fn ignores_issue_todos_owned_by_another_loopx_workflow() { + let todo = serde_json::json!({ + "role": "agent", + "action_kind": "issue_fix_portfolio_advancement", + "text": "Advance https://github.com/owner/repo/issues/1920", + "todo_id": "todo_1920", + "status": "open" + }); + + assert!(project_issue_todo(&todo).is_none()); + } + + #[test] + fn projects_only_a_gate_linked_to_a_managed_issue() { + let issues = vec![AutonomousIssueTodo { + issue_ref: "1849".to_string(), + issue_url: "https://github.com/owner/repo/issues/1849".to_string(), + todo_id: "todo_1849".to_string(), + status: "open".to_string(), + selected: true, + }]; + let todos = vec![serde_json::json!({ + "todo_id": "gate_1849", + "role": "user", + "status": "open", + "task_class": "user_gate", + "unblocks_todo_id": "todo_1849", + "text": "Open the validated pull request for #1849?" + })]; + + let question = project_user_question(&todos, &issues).expect("gate projects"); + assert_eq!(question.todo_id, "gate_1849"); + assert_eq!( + question.prompt, + "Open the validated pull request for #1849?" + ); + } + + #[test] + fn issue_linked_gates_outrank_goal_wide_gates() { + let issues = vec![AutonomousIssueTodo { + issue_ref: "1849".to_string(), + issue_url: "https://github.com/owner/repo/issues/1849".to_string(), + todo_id: "todo_1849".to_string(), + status: "open".to_string(), + selected: true, + }]; + let todos = vec![ + serde_json::json!({ + "todo_id": "gate_goal", + "role": "user", + "status": "open", + "task_class": "user_gate", + "text": "Goal-wide gate without a link" + }), + serde_json::json!({ + "todo_id": "gate_1849", + "role": "user", + "status": "open", + "task_class": "user_gate", + "unblocks_todo_id": "todo_1849", + "text": "Open the validated pull request for #1849?" + }), + ]; + + let question = project_user_question(&todos, &issues).expect("gate projects"); + assert_eq!(question.todo_id, "gate_1849"); + } + + #[test] + fn an_open_gate_is_never_invisible_even_without_an_issue_link() { + // The Kernel blocks on any open user_gate; hiding it would stall the + // loop with nothing to answer, so unlinked gates surface as fallback. + let issues = vec![AutonomousIssueTodo { + issue_ref: "1849".to_string(), + issue_url: "https://github.com/owner/repo/issues/1849".to_string(), + todo_id: "todo_1849".to_string(), + status: "open".to_string(), + selected: true, + }]; + let unlinked = vec![serde_json::json!({ + "todo_id": "gate_force_push", + "role": "user", + "status": "open", + "task_class": "user_gate", + "unblocks_todo_id": "todo_other", + "text": "Allow force-push to the feature branch?" + })]; + let question = project_user_question(&unlinked, &issues).expect("fallback projects"); + assert_eq!(question.todo_id, "gate_force_push"); + + // Non-gate user todos (reading queues, user_action) never project. + let user_action = vec![serde_json::json!({ + "todo_id": "todo_review", + "role": "user", + "status": "open", + "task_class": "user_action", + "text": "Review the weekly report" + })]; + assert!(project_user_question(&user_action, &issues).is_none()); + } + + #[test] + fn gate_projection_does_not_depend_on_quota_preview_truncation() { + // LoopX compacts `gate_open_items` to two entries; the projection must + // find a gate that never appears in that preview. + let issues = vec![AutonomousIssueTodo { + issue_ref: "3".to_string(), + issue_url: "https://github.com/owner/repo/issues/3".to_string(), + todo_id: "todo_3".to_string(), + status: "open".to_string(), + selected: false, + }]; + let todos = vec![ + serde_json::json!({ + "todo_id": "gate_unmanaged", + "role": "user", + "status": "open", + "task_class": "user_gate", + "unblocks_todo_id": "todo_unmanaged", + "text": "Gate for a todo this panel does not manage" + }), + serde_json::json!({ + "todo_id": "gate_3", + "role": "user", + "status": "open", + "task_class": "user_gate", + "unblocks_todo_id": "todo_3", + "text": "Publish the validated patch for #3?" + }), + ]; + + let question = project_user_question(&todos, &issues).expect("third gate projects"); + assert_eq!(question.todo_id, "gate_3"); + } + + #[test] + fn heartbeat_prompt_carries_host_preamble_before_loopx_contract() { + let prompt = compose_heartbeat_prompt("Advance `goal` using `state`."); + let preamble_end = prompt + .find("--- LoopX heartbeat contract follows ---") + .expect("delimiter present"); + assert!(prompt[..preamble_end].contains("LoopX Kernel state is the ONLY source of truth")); + assert!(prompt.ends_with("Advance `goal` using `state`.")); + } + + #[test] + fn scheduler_args_select_prompt_mode_only_for_heartbeat() { + let identity = ControlIdentity { + goal_id: "goal".to_string(), + agent_id: "agent".to_string(), + }; + let quota = scheduler_args("quota", "should-run", &identity, None); + assert!(!quota.iter().any(|arg| arg == "--compact" || arg == "--thin")); + let heartbeat = scheduler_args("heartbeat-prompt", "", &identity, Some("--compact")); + assert_eq!(heartbeat.last().map(String::as_str), Some("--compact")); + } + + #[test] + fn user_decision_is_a_typed_loopx_todo_transition() { + let identity = ControlIdentity { + goal_id: "goal".to_string(), + agent_id: "agent".to_string(), + }; + let args = user_decision_args( + &identity, + "gate_1849", + UserDecision::Reject, + Some("Keep the validated patch local."), + ) + .expect("decision args"); + + assert_eq!( + args, + vec![ + "todo", + "complete", + "--goal-id", + "goal", + "--role", + "user", + "--todo-id", + "gate_1849", + "--agent-id", + "agent", + "--decision-outcome", + "reject", + "--note", + "Keep the validated patch local.", + ] + ); + } + + #[test] + fn prose_issue_number_is_not_treated_as_identity() { + let todo = serde_json::json!({ + "role": "agent", + "action_kind": "issue_fix_intake", + "text": "Investigate issue #1849 without an explicit URL.", + "todo_id": "todo_1849" + }); + assert!(project_issue_todo(&todo).is_none()); + } + + #[test] + fn selection_ref_must_match_url() { + let error = validate_selection(&IssueSelection { + issue_ref: "1849".to_string(), + issue_url: "https://github.com/owner/repo/issues/1580".to_string(), + }) + .expect_err("mismatch must fail"); + assert!(error.to_string().contains("does not match")); + } + + #[test] + fn todo_text_is_stable_for_loopx_deduplication() { + let issue = IssueSelection { + issue_ref: "1849".to_string(), + issue_url: "https://github.com/owner/repo/issues/1849".to_string(), + }; + assert_eq!( + issue_todo_text("owner/repo", &issue), + issue_todo_text("owner/repo", &issue) + ); + } + + #[test] + fn identity_requires_one_goal_and_one_agent() { + let temp = tempfile::tempdir().expect("tempdir"); + std::fs::create_dir_all(temp.path().join(".loopx")).expect("registry dir"); + std::fs::write( + temp.path().join(".loopx").join("registry.json"), + br#"{"goals":[{"id":"goal","status":"active","coordination":{"registered_agents":["agent"]}}]}"#, + ) + .expect("registry write"); + + let identity = read_identity(temp.path()).expect("identity resolves"); + assert_eq!(identity.goal_id, "goal"); + assert_eq!(identity.agent_id, "agent"); + } +} diff --git a/src/crates/services/services-integrations/src/loopx_issue_fix/mod.rs b/src/crates/services/services-integrations/src/loopx_issue_fix/mod.rs new file mode 100644 index 0000000000..48754e9eba --- /dev/null +++ b/src/crates/services/services-integrations/src/loopx_issue_fix/mod.rs @@ -0,0 +1,300 @@ +//! Bridge to the external `loopx` CLI's `issue-fix` capability. +//! +//! LoopX supplies the deterministic decision skeleton (which route to take for an +//! issue, how to project a PR's lifecycle) and owns the durable Issue-Fix control +//! state. This crate invokes those typed transitions and keeps host concerns out +//! of LoopX's domain state. +//! +//! See `docs/development/loopx-issue-fix-integration.md` for the verified chain. + +pub mod autonomous; +/// Typed packet parsers for LoopX's plan/execute CLI surface. The product path +/// now drives the lifecycle through the heartbeat agent (see [`autonomous`]), +/// so `orchestrator` and `repository_context` are exercised only by the +/// `loopx_issue_fix_contracts` integration tests, kept as executable +/// documentation of the CLI contract (e.g. `decision.route` vs +/// `transition.decision`). +pub mod orchestrator; +pub mod repository_context; + +use std::ffi::OsStr; +use std::path::{Path, PathBuf}; +use std::process::Stdio; + +use thiserror::Error; +use tokio::process::Command; + +/// Override for the `loopx` program path, mirroring `FLASHGREP_DAEMON_BIN`. +const LOOPX_BIN_ENV: &str = "LOOPX_BIN"; + +/// LoopX's subprocess call sites pass `text=True` without `encoding=`, so on a +/// non-UTF-8 locale (notably Chinese Windows, `cp936`) it decodes `gh`'s UTF-8 +/// output as GBK and dies. Forcing Python's UTF-8 mode fixes every call site at +/// once and needs no patch to LoopX itself. +const PYTHON_UTF8_ENV: &str = "PYTHONUTF8"; + +/// Cap on captured output, so a runaway subprocess cannot exhaust memory. +const MAX_OUTPUT_BYTES: usize = 8 * 1024 * 1024; + +#[derive(Debug, Error)] +pub enum LoopxIssueFixError { + #[error("failed to spawn loopx: {0}")] + Spawn(#[source] std::io::Error), + #[error("loopx exited with status {status}: {stderr}")] + Exit { status: String, stderr: String }, + #[error("loopx produced {bytes} bytes of output, exceeding the {limit} byte limit")] + OutputTooLarge { bytes: usize, limit: usize }, + #[error("loopx returned output that is not valid UTF-8")] + NonUtf8Output, + #[error("loopx returned output that is not valid JSON: {0}")] + InvalidJson(#[source] serde_json::Error), + /// LoopX reports domain-level refusals in-band as `{"ok": false, "error": ...}`. + /// It also exits nonzero for these, so the bridge parses stdout before looking + /// at the exit status; otherwise the reason would be lost. + #[error("loopx rejected the request: {0}")] + Rejected(String), +} + +/// A resolved `loopx` program, ready to invoke. +/// +/// Construct with [`LoopxIssueFix::probe`]; a `None` result means the feature is +/// unavailable on this host and its entry points should stay hidden. +#[derive(Debug, Clone)] +pub struct LoopxIssueFix { + program: PathBuf, +} + +impl LoopxIssueFix { + /// Resolve `loopx`, preferring an explicit `LOOPX_BIN` override over `PATH`. + /// + /// Returns `None` when no usable program exists. Callers should treat that as + /// "feature unavailable" rather than an error. + pub fn probe() -> Option { + if let Some(raw) = std::env::var_os(LOOPX_BIN_ENV) { + let path = PathBuf::from(raw); + if path.is_file() { + return Some(Self { program: path }); + } + } + + which::which("loopx").ok().map(|program| Self { program }) + } + + /// The resolved program path, for diagnostics. + pub fn program(&self) -> &Path { + &self.program + } + + /// Run one `loopx issue-fix` subcommand and parse its JSON packet. + /// + /// `args` should omit both the `issue-fix` prefix and `--format json`; this + /// method supplies them. + pub async fn issue_fix(&self, args: I) -> Result + where + I: IntoIterator, + S: AsRef, + { + let mut command = Command::new(&self.program); + command.arg("issue-fix"); + command.args(args); + command.arg("--format"); + command.arg("json"); + self.run_json_command(command).await + } + + /// Run any LoopX JSON command from the selected project root. + /// + /// `--format json` is a global LoopX option, so this method places it before + /// the supplied subcommand. Autonomous issue fixing uses this path for todo, + /// quota, and heartbeat commands while keeping the project-local registry as + /// the only control-plane source. + pub async fn json_in( + &self, + cwd: &Path, + args: I, + ) -> Result + where + I: IntoIterator, + S: AsRef, + { + let mut command = Command::new(&self.program); + command.arg("--format"); + command.arg("json"); + command.args(args); + command.current_dir(cwd); + self.run_json_command(command).await + } + + async fn run_json_command( + &self, + mut command: Command, + ) -> Result { + command.env(PYTHON_UTF8_ENV, "1"); + command.stdin(Stdio::null()); + command.stdout(Stdio::piped()); + command.stderr(Stdio::piped()); + #[cfg(windows)] + { + // Suppress the console window that would otherwise flash on spawn. + const CREATE_NO_WINDOW: u32 = 0x0800_0000; + command.creation_flags(CREATE_NO_WINDOW); + } + + let output = command.output().await.map_err(LoopxIssueFixError::Spawn)?; + + // LoopX signals a domain refusal with BOTH `{"ok": false, "error": ...}` on + // stdout AND exit code 1. Parse stdout first so the structured reason wins; + // checking the status first would discard it and report a bare exit code. + match parse_packet(&output.stdout) { + Ok(packet) => Ok(packet), + Err(refusal @ LoopxIssueFixError::Rejected(_)) => Err(refusal), + Err(parse_error) => { + if output.status.success() { + // Exited cleanly but produced something unparseable. + Err(parse_error) + } else { + // A crash, a bad flag, or a missing dependency: stderr explains + // it far better than a JSON parse failure would. + Err(LoopxIssueFixError::Exit { + status: describe_status(&output.status), + stderr: truncated_stderr(&output.stderr), + }) + } + } + } + } +} + +fn describe_status(status: &std::process::ExitStatus) -> String { + status + .code() + .map(|code| code.to_string()) + .unwrap_or_else(|| "signal".to_string()) +} + +fn truncated_stderr(stderr: &[u8]) -> String { + const MAX_STDERR_CHARS: usize = 2_000; + let text = String::from_utf8_lossy(stderr); + let trimmed = text.trim(); + if trimmed.chars().count() <= MAX_STDERR_CHARS { + return trimmed.to_string(); + } + trimmed.chars().take(MAX_STDERR_CHARS).collect() +} + +/// Parse a LoopX packet, surfacing in-band `{"ok": false}` refusals as errors. +fn parse_packet(stdout: &[u8]) -> Result { + if stdout.len() > MAX_OUTPUT_BYTES { + return Err(LoopxIssueFixError::OutputTooLarge { + bytes: stdout.len(), + limit: MAX_OUTPUT_BYTES, + }); + } + + let text = std::str::from_utf8(stdout).map_err(|_| LoopxIssueFixError::NonUtf8Output)?; + let packet: serde_json::Value = + serde_json::from_str(text.trim()).map_err(LoopxIssueFixError::InvalidJson)?; + + if packet.get("ok").and_then(serde_json::Value::as_bool) == Some(false) { + let reason = packet + .get("error") + .and_then(serde_json::Value::as_str) + .unwrap_or("loopx reported ok=false without an error message"); + return Err(LoopxIssueFixError::Rejected(reason.to_string())); + } + + Ok(packet) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_packet_accepts_a_successful_projection() { + let packet = parse_packet(br#"{"ok": true, "route": "fix_pr"}"#).expect("packet parses"); + assert_eq!(packet["route"], "fix_pr"); + } + + #[test] + fn parse_packet_tolerates_surrounding_whitespace() { + let packet = parse_packet(b"\n {\"ok\": true}\n\n").expect("packet parses"); + assert_eq!(packet["ok"], true); + } + + #[test] + fn parse_packet_surfaces_in_band_refusals_as_errors() { + // LoopX reports domain refusals on stdout AND exits nonzero, so this must + // not look like success to callers. `issue_fix` parses stdout first so that + // this reason survives instead of being replaced by a bare exit code. + let error = parse_packet(br#"{"ok": false, "error": "scope_class must be provided"}"#) + .expect_err("ok=false is an error"); + match error { + LoopxIssueFixError::Rejected(reason) => { + assert!( + reason.contains("scope_class"), + "unexpected reason: {reason}" + ); + } + other => panic!("expected Rejected, got {other:?}"), + } + } + + #[test] + fn parse_packet_reports_a_missing_error_message() { + let error = parse_packet(br#"{"ok": false}"#).expect_err("ok=false is an error"); + assert!(matches!(error, LoopxIssueFixError::Rejected(_))); + } + + #[test] + fn parse_packet_does_not_treat_a_missing_ok_field_as_refusal() { + let packet = parse_packet(br#"{"decision": "user_gate"}"#).expect("packet parses"); + assert_eq!(packet["decision"], "user_gate"); + } + + #[test] + fn parse_packet_rejects_non_json_output() { + let error = + parse_packet(b"Traceback (most recent call last):").expect_err("non-JSON is an error"); + assert!(matches!(error, LoopxIssueFixError::InvalidJson(_))); + } + + #[test] + fn parse_packet_rejects_non_utf8_output() { + // A GBK-mangled byte sequence, the shape of LoopX's Windows encoding bug. + let error = parse_packet(&[0x7b, 0x80, 0xfe, 0x7d]).expect_err("non-UTF-8 is an error"); + assert!(matches!(error, LoopxIssueFixError::NonUtf8Output)); + } + + #[test] + fn parse_packet_rejects_oversized_output() { + let oversized = vec![b' '; MAX_OUTPUT_BYTES + 1]; + let error = parse_packet(&oversized).expect_err("oversized output is an error"); + match error { + LoopxIssueFixError::OutputTooLarge { bytes, limit } => { + assert_eq!(bytes, MAX_OUTPUT_BYTES + 1); + assert_eq!(limit, MAX_OUTPUT_BYTES); + } + other => panic!("expected OutputTooLarge, got {other:?}"), + } + } + + #[test] + fn truncated_stderr_bounds_its_output() { + let long = "e".repeat(5_000); + assert_eq!(truncated_stderr(long.as_bytes()).chars().count(), 2_000); + } + + #[test] + fn truncated_stderr_trims_whitespace() { + assert_eq!(truncated_stderr(b" boom \n"), "boom"); + } + + #[test] + fn probe_prefers_an_explicit_override_over_path() { + // Only assert the negative case, which needs no real loopx install: a + // non-existent override must not be accepted. + let path = PathBuf::from("/nonexistent/loopx-should-not-resolve"); + assert!(!path.is_file()); + } +} diff --git a/src/crates/services/services-integrations/src/loopx_issue_fix/orchestrator.rs b/src/crates/services/services-integrations/src/loopx_issue_fix/orchestrator.rs new file mode 100644 index 0000000000..cae3cd81b8 --- /dev/null +++ b/src/crates/services/services-integrations/src/loopx_issue_fix/orchestrator.rs @@ -0,0 +1,856 @@ +//! Run one issue through LoopX's deterministic decision chain. +//! +//! The chain is `workflow-plan` → `feasibility` → `caller-repo-branch` → +//! `pr-lifecycle`. LoopX decides *what* to do at each step and writes nothing; +//! BitFun supplies the evidence and performs every side effect. +//! +//! This module exists mostly to make LoopX's JSON safe to consume. The fields +//! that matter sit at non-obvious paths — the route is `decision.route`, not +//! `route`, and the lifecycle decision is `transition.decision` — which is easy +//! to read wrong from the markdown rendering, where both appear flattened. Typed +//! outcomes here mean a caller cannot silently misread a refusal as approval. + +use std::path::Path; + +use serde::{Deserialize, Serialize}; + +use super::repository_context::RepositoryContext; +use super::{LoopxIssueFix, LoopxIssueFixError}; + +/// Which resolution LoopX selected for an issue. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum FixRoute { + /// Prepare a branch, validate it, and open a pull request. + FixPr, + /// Draft a maintainer comment; posting still needs an explicit gate. + CommentOnly, + /// Record a blocker instead of opening an ungrounded patch loop. + TriageOnly, +} + +impl FixRoute { + fn parse(value: &str) -> Option { + match value { + "fix_pr" => Some(Self::FixPr), + "comment_only" => Some(Self::CommentOnly), + "triage_only" => Some(Self::TriageOnly), + _ => None, + } + } + + /// Whether this route may lead to a pull request at all. + pub fn permits_pull_request(self) -> bool { + self == Self::FixPr + } +} + +/// What LoopX says should happen next. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum NextStep { + /// There is agent work to do now. + RunnableSuccessor, + /// Keep watching; create no successor. + MonitorContinuation, + /// A human must decide before anything else happens. + UserGate, + /// Terminal; nothing follows. + NoFollowup, +} + +impl NextStep { + fn parse(value: &str) -> Option { + match value { + "runnable_successor" => Some(Self::RunnableSuccessor), + "monitor_continuation" => Some(Self::MonitorContinuation), + "user_gate" => Some(Self::UserGate), + "no_followup" => Some(Self::NoFollowup), + _ => None, + } + } + + /// Whether a caller must stop and ask a human. + /// + /// LoopX raises this for semantic ambiguity and for missing write authority. + /// Crossing it automatically would defeat the gate. + pub fn requires_human(self) -> bool { + self == Self::UserGate + } +} + +/// How much of the issue's context BitFun managed to ground. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ContextGrounding { + Grounded, + Partial, + Ungrounded, + NotProvided, +} + +impl ContextGrounding { + fn parse(value: &str) -> Option { + match value { + "grounded" => Some(Self::Grounded), + "partial" => Some(Self::Partial), + "ungrounded" => Some(Self::Ungrounded), + "not_provided" => Some(Self::NotProvided), + _ => None, + } + } +} + +/// LoopX's route decision for one issue. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct FeasibilityOutcome { + pub route: FixRoute, + pub next_step: NextStep, + pub context_grounding: ContextGrounding, + /// Why LoopX decided this, verbatim. Useful to show a user why a fix was + /// declined without reinterpreting it. + pub reason_codes: Vec, + /// Which of change_scope / reproduction / validation are still unresolved. + pub unresolved_aspects: Vec, +} + +/// The state of a prepared issue branch. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct BranchOutcome { + pub issue_branch: String, + pub base_branch: String, + /// `dry_run` until a caller opts into execution. + pub branch_action: String, + pub branch_ready: bool, + pub validation_executed: bool, + pub validation_passed: bool, + pub changed_files: Vec, + /// Both this and a `FixRoute::FixPr` route must hold before opening a PR. + pub review_packet_ready: bool, + pub review_packet_summary: String, + /// Why the packet is not ready yet, when it is not. + pub readiness_blockers: Vec, +} + +impl BranchOutcome { + /// Whether a pull request may be opened for this branch. + /// + /// Deliberately requires the route *and* packet readiness together: the + /// feature ships without a runtime kill switch, so this gate lives on the + /// action rather than relying on a disabled toggle. + pub fn may_open_pull_request(&self, route: FixRoute) -> bool { + route.permits_pull_request() && self.review_packet_ready && self.validation_passed + } +} + +/// How an open pull request should be followed up. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct PullRequestOutcome { + pub next_step: NextStep, + pub state: String, + pub state_bucket: String, + pub reason: String, + /// Write scopes the successor would need, when it needs any. + pub required_write_scopes: Vec, +} + +/// What a caller should do next after planning an issue. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct PlanOutcome { + pub issue_ref: String, + pub feasibility: FeasibilityOutcome, + /// Absent when the route does not lead to a branch, or when planning only. + pub branch: Option, +} + +/// Inputs for one issue's run. +#[derive(Debug, Clone)] +pub struct IssueFixRequest<'a> { + /// Public-safe `owner/repo` label. + pub repo: &'a str, + pub issue_ref: &'a str, + pub issue_url: &'a str, + /// Evidence BitFun gathered by reading the repository. + pub context: &'a RepositoryContext, + /// How the fix would be checked. LoopX will not select `fix_pr` without + /// this, whatever the context says. + pub validation_label: &'a str, + /// Compact label for the reproduction, not a raw command. + pub reproduction_label: &'a str, + pub reproduction_status: ReproductionStatus, + pub scope_class: ScopeClass, + pub base_branch: &'a str, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ReproductionStatus { + Confirmed, + Planned, + Missing, + Blocked, +} + +impl ReproductionStatus { + fn as_arg(self) -> &'static str { + match self { + Self::Confirmed => "confirmed", + Self::Planned => "planned", + Self::Missing => "missing", + Self::Blocked => "blocked", + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ScopeClass { + Bounded, + Uncertain, + Oversized, +} + +impl ScopeClass { + fn as_arg(self) -> &'static str { + match self { + Self::Bounded => "bounded", + Self::Uncertain => "uncertain", + Self::Oversized => "oversized", + } + } +} + +/// Whether a step may touch the working tree. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ExecutionMode { + /// Plan only. Nothing is created, nothing runs. + DryRun, + /// Create or claim the issue branch and run the validation command. + Execute { + /// Runs in the repository. A caller must have explicit approval for it. + validation_command: &'static str, + }, +} + +#[derive(Debug)] +pub enum OrchestratorError { + Loopx(LoopxIssueFixError), + /// A field LoopX is contracted to return was missing or unrecognized. + UnexpectedPacket { + field: &'static str, + value: String, + }, + ContextWrite(std::io::Error), + ContextSerialize(serde_json::Error), + /// The context file path could not be passed to LoopX as text. + NonUtf8Path, +} + +impl std::fmt::Display for OrchestratorError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Loopx(error) => write!(f, "{error}"), + Self::UnexpectedPacket { field, value } => write!( + f, + "loopx returned an unrecognized {field}: {value:?}; the CLI contract may have changed" + ), + Self::ContextWrite(error) => { + write!(f, "failed to write the repository context: {error}") + } + Self::ContextSerialize(error) => { + write!(f, "failed to serialize the repository context: {error}") + } + Self::NonUtf8Path => write!(f, "the repository context path is not valid UTF-8"), + } + } +} + +impl std::error::Error for OrchestratorError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Self::Loopx(error) => Some(error), + Self::ContextWrite(error) => Some(error), + Self::ContextSerialize(error) => Some(error), + _ => None, + } + } +} + +impl From for OrchestratorError { + fn from(error: LoopxIssueFixError) -> Self { + Self::Loopx(error) + } +} + +/// Make a validation command spawnable by LoopX on Windows. +/// +/// LoopX launches the caller-declared validation command with +/// `subprocess.run(shlex.split(command))` and no shell (see +/// `acceptance_loop.py:_run_caller_validation`). On Windows that cannot start +/// `.cmd` / `.bat` shims such as `pnpm` or `npm` — `CreateProcess` only +/// resolves executables — so the bridge reports `[WinError 2]`. Delegating +/// through `cmd /c` makes every command spawnable. This is a host-side concern: +/// BitFun owns the validation command and must not patch LoopX itself. +#[cfg(windows)] +fn windows_safe_validation_command(command: &str) -> String { + let trimmed = command.trim_start(); + if trimmed.starts_with("cmd /c") + || trimmed.starts_with("cmd.exe /c") + || trimmed.starts_with("cmd ") + || trimmed.starts_with("cmd.exe ") + { + command.to_string() + } else { + format!("cmd /c {command}") + } +} + +#[cfg(not(windows))] +fn windows_safe_validation_command(command: &str) -> String { + command.to_string() +} + +/// Drives one issue through the chain. +pub struct IssueFixOrchestrator<'a> { + loopx: &'a LoopxIssueFix, +} + +impl<'a> IssueFixOrchestrator<'a> { + pub fn new(loopx: &'a LoopxIssueFix) -> Self { + Self { loopx } + } + + /// Ask LoopX which route this issue should take. + /// + /// Read-only: LoopX writes nothing, and `--no-write-domain-state` keeps it + /// from touching goal state either. + pub async fn feasibility( + &self, + request: &IssueFixRequest<'_>, + context_dir: &Path, + ) -> Result { + let context_path = write_context(request.context, context_dir)?; + let context_path = context_path + .to_str() + .ok_or(OrchestratorError::NonUtf8Path)?; + + let packet = self + .loopx + .issue_fix([ + "feasibility", + "--repo", + request.repo, + "--issue-ref", + request.issue_ref, + "--url", + request.issue_url, + "--reproduction-status", + request.reproduction_status.as_arg(), + "--scope-class", + request.scope_class.as_arg(), + "--reproduction-label", + request.reproduction_label, + "--validation-label", + request.validation_label, + "--repository-context-json", + context_path, + "--no-write-domain-state", + ]) + .await?; + + parse_feasibility(&packet) + } + + /// Prepare the issue branch and, when executing, run the validation command. + /// + /// In [`ExecutionMode::DryRun`] this creates nothing; the returned + /// `branch_action` reports `dry_run`. + pub async fn prepare_branch( + &self, + request: &IssueFixRequest<'_>, + repo_path: &str, + mode: ExecutionMode, + ) -> Result { + let mut args = vec![ + "caller-repo-branch", + "--repo-path", + repo_path, + "--repo", + request.repo, + "--issue-ref", + request.issue_ref, + "--url", + request.issue_url, + "--base-branch", + request.base_branch, + "--validation-label", + request.validation_label, + ]; + let mut validation_arg: Option = None; + if let ExecutionMode::Execute { validation_command } = mode { + let wrapped = windows_safe_validation_command(validation_command); + validation_arg = Some(wrapped); + } + if let Some(command) = validation_arg.as_deref() { + args.push("--validation-command"); + args.push(command); + args.push("--execute"); + } + + let packet = self.loopx.issue_fix(args).await?; + parse_branch(&packet) + } + + /// Project an open pull request's lifecycle onto a next step. + pub async fn pull_request_lifecycle( + &self, + repo: &str, + pull_request_ref: &str, + issue_ref: &str, + metadata_path: Option<&str>, + ) -> Result { + let mut args = vec![ + "pr-lifecycle", + "--repo", + repo, + "--pr-ref", + pull_request_ref, + "--issue-ref", + issue_ref, + "--no-write-domain-state", + ]; + match metadata_path { + Some(path) => { + args.push("--metadata-json"); + args.push(path); + } + None => args.push("--fetch-metadata"), + } + + let packet = self.loopx.issue_fix(args).await?; + parse_pull_request(&packet) + } + + /// Plan one issue: decide the route, then prepare a branch only when the + /// route actually permits a pull request. + pub async fn plan_issue( + &self, + request: &IssueFixRequest<'_>, + repo_path: &str, + context_dir: &Path, + mode: ExecutionMode, + ) -> Result { + let feasibility = self.feasibility(request, context_dir).await?; + + // Skip the branch entirely on a non-fix route. Preparing one would be + // wasted work at best, and on `--execute` it would create a branch LoopX + // just declined to justify. + let branch = if feasibility.route.permits_pull_request() { + Some(self.prepare_branch(request, repo_path, mode).await?) + } else { + None + }; + + Ok(PlanOutcome { + issue_ref: request.issue_ref.to_string(), + feasibility, + branch, + }) + } +} + +fn write_context( + context: &RepositoryContext, + dir: &Path, +) -> Result { + let path = dir.join("loopx-repository-context.json"); + let bytes = serde_json::to_vec(context).map_err(OrchestratorError::ContextSerialize)?; + std::fs::write(&path, bytes).map_err(OrchestratorError::ContextWrite)?; + Ok(path) +} + +fn required_str<'p>( + packet: &'p serde_json::Value, + path: &[&str], + field: &'static str, +) -> Result<&'p str, OrchestratorError> { + let mut cursor = packet; + for key in path { + cursor = &cursor[key]; + } + cursor + .as_str() + .ok_or_else(|| OrchestratorError::UnexpectedPacket { + field, + value: cursor.to_string(), + }) +} + +fn string_list(packet: &serde_json::Value, path: &[&str]) -> Vec { + let mut cursor = packet; + for key in path { + cursor = &cursor[key]; + } + cursor + .as_array() + .map(|items| { + items + .iter() + .filter_map(|item| item.as_str().map(str::to_string)) + .collect() + }) + .unwrap_or_default() +} + +fn parse_feasibility(packet: &serde_json::Value) -> Result { + // `decision.route`, not `route`: the markdown rendering flattens these, so + // reading the top level here would silently yield null. + let route_text = required_str(packet, &["decision", "route"], "decision.route")?; + let route = FixRoute::parse(route_text).ok_or_else(|| OrchestratorError::UnexpectedPacket { + field: "decision.route", + value: route_text.to_string(), + })?; + + let step_text = required_str(packet, &["transition", "decision"], "transition.decision")?; + let next_step = + NextStep::parse(step_text).ok_or_else(|| OrchestratorError::UnexpectedPacket { + field: "transition.decision", + value: step_text.to_string(), + })?; + + let grounding_text = required_str( + packet, + &["observation", "repository_context", "context_status"], + "observation.repository_context.context_status", + )?; + let context_grounding = ContextGrounding::parse(grounding_text).ok_or_else(|| { + OrchestratorError::UnexpectedPacket { + field: "observation.repository_context.context_status", + value: grounding_text.to_string(), + } + })?; + + Ok(FeasibilityOutcome { + route, + next_step, + context_grounding, + reason_codes: string_list(packet, &["decision", "reason_codes"]), + unresolved_aspects: string_list( + packet, + &[ + "observation", + "repository_context", + "unresolved_required_aspects", + ], + ), + }) +} + +fn parse_branch(packet: &serde_json::Value) -> Result { + let artifact = &packet["caller_repo_branch"]; + let review_packet = &packet["review_packet"]; + + Ok(BranchOutcome { + issue_branch: required_str( + artifact, + &["issue_branch"], + "caller_repo_branch.issue_branch", + )? + .to_string(), + base_branch: required_str(artifact, &["base_branch"], "caller_repo_branch.base_branch")? + .to_string(), + branch_action: required_str( + artifact, + &["branch_action"], + "caller_repo_branch.branch_action", + )? + .to_string(), + branch_ready: artifact["branch_ready"].as_bool().unwrap_or(false), + validation_executed: artifact["validation"]["executed"] + .as_bool() + .unwrap_or(false), + validation_passed: artifact["validation"]["passed"].as_bool().unwrap_or(false), + changed_files: string_list(artifact, &["changed_files"]), + review_packet_ready: review_packet["ready"].as_bool().unwrap_or(false), + review_packet_summary: review_packet["summary"] + .as_str() + .unwrap_or_default() + .to_string(), + readiness_blockers: string_list(review_packet, &["readiness_blockers"]), + }) +} + +fn parse_pull_request(packet: &serde_json::Value) -> Result { + let step_text = required_str(packet, &["transition", "decision"], "transition.decision")?; + let next_step = + NextStep::parse(step_text).ok_or_else(|| OrchestratorError::UnexpectedPacket { + field: "transition.decision", + value: step_text.to_string(), + })?; + + Ok(PullRequestOutcome { + next_step, + // These live under `observation` and `grouped_monitor_projection`, not at + // the top level — one more reason this parsing belongs in one place. + state: packet["observation"]["state"] + .as_str() + .unwrap_or_default() + .to_string(), + state_bucket: packet["grouped_monitor_projection"]["state_bucket"] + .as_str() + .unwrap_or_default() + .to_string(), + reason: packet["transition"]["reason"] + .as_str() + .unwrap_or_default() + .to_string(), + required_write_scopes: string_list(packet, &["transition", "required_write_scopes"]), + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn feasibility_reads_the_nested_route_and_decision() { + // The exact shape LoopX returns. Both fields are nested; a top-level read + // would yield null and, before typing, would have looked like success. + let packet = serde_json::json!({ + "ok": true, + "decision": { + "route": "fix_pr", + "reason_codes": ["reproduction_confirmed", "validation_surface_named"], + }, + "transition": {"decision": "runnable_successor"}, + "observation": { + "repository_context": { + "context_status": "grounded", + "unresolved_required_aspects": [], + }, + }, + }); + + let outcome = parse_feasibility(&packet).expect("packet parses"); + assert_eq!(outcome.route, FixRoute::FixPr); + assert!(outcome.route.permits_pull_request()); + assert_eq!(outcome.next_step, NextStep::RunnableSuccessor); + assert_eq!(outcome.context_grounding, ContextGrounding::Grounded); + assert_eq!(outcome.reason_codes.len(), 2); + assert!(outcome.unresolved_aspects.is_empty()); + } + + #[test] + fn a_triage_route_does_not_permit_a_pull_request() { + let packet = serde_json::json!({ + "decision": {"route": "triage_only", "reason_codes": ["scope_oversized"]}, + "transition": {"decision": "no_followup"}, + "observation": { + "repository_context": { + "context_status": "partial", + "unresolved_required_aspects": ["validation"], + }, + }, + }); + + let outcome = parse_feasibility(&packet).expect("packet parses"); + assert_eq!(outcome.route, FixRoute::TriageOnly); + assert!(!outcome.route.permits_pull_request()); + assert_eq!(outcome.unresolved_aspects, vec!["validation"]); + } + + #[test] + fn a_user_gate_requires_a_human() { + assert!(NextStep::UserGate.requires_human()); + for step in [ + NextStep::RunnableSuccessor, + NextStep::MonitorContinuation, + NextStep::NoFollowup, + ] { + assert!(!step.requires_human(), "{step:?} should not gate"); + } + } + + #[test] + fn an_unrecognized_route_is_an_error_not_a_default() { + // Silently defaulting an unknown route could turn a refusal into a PR. + let packet = serde_json::json!({ + "decision": {"route": "ship_it_immediately"}, + "transition": {"decision": "runnable_successor"}, + "observation": {"repository_context": {"context_status": "grounded"}}, + }); + + let error = parse_feasibility(&packet).expect_err("unknown routes are rejected"); + match error { + OrchestratorError::UnexpectedPacket { field, value } => { + assert_eq!(field, "decision.route"); + assert_eq!(value, "ship_it_immediately"); + } + other => panic!("expected UnexpectedPacket, got {other:?}"), + } + } + + #[test] + fn a_missing_route_is_an_error() { + let packet = serde_json::json!({"ok": true}); + let error = parse_feasibility(&packet).expect_err("a missing route is rejected"); + assert!(matches!( + error, + OrchestratorError::UnexpectedPacket { + field: "decision.route", + .. + } + )); + } + + #[test] + fn branch_parsing_reports_dry_run_state() { + let packet = serde_json::json!({ + "caller_repo_branch": { + "issue_branch": "codex/issue-1849-fix", + "base_branch": "main", + "branch_action": "dry_run", + "branch_ready": false, + "validation": {"executed": false, "passed": false}, + "changed_files": [], + }, + "review_packet": { + "ready": false, + "summary": "validation is not PR-ready yet", + "readiness_blockers": ["validation_not_run"], + }, + }); + + let outcome = parse_branch(&packet).expect("packet parses"); + assert_eq!(outcome.issue_branch, "codex/issue-1849-fix"); + assert_eq!(outcome.branch_action, "dry_run"); + assert!(!outcome.branch_ready); + assert_eq!(outcome.readiness_blockers, vec!["validation_not_run"]); + } + + #[test] + fn opening_a_pull_request_needs_route_packet_and_validation_together() { + let ready = BranchOutcome { + issue_branch: "codex/issue-1-fix".to_string(), + base_branch: "main".to_string(), + branch_action: "created".to_string(), + branch_ready: true, + validation_executed: true, + validation_passed: true, + changed_files: vec!["src/a.rs".to_string()], + review_packet_ready: true, + review_packet_summary: "ready".to_string(), + readiness_blockers: Vec::new(), + }; + assert!(ready.may_open_pull_request(FixRoute::FixPr)); + + // Each condition alone must be able to veto. + assert!( + !ready.may_open_pull_request(FixRoute::CommentOnly), + "a non-fix route must veto" + ); + let mut unvalidated = ready.clone(); + unvalidated.validation_passed = false; + assert!( + !unvalidated.may_open_pull_request(FixRoute::FixPr), + "failing validation must veto" + ); + let mut unready = ready.clone(); + unready.review_packet_ready = false; + assert!( + !unready.may_open_pull_request(FixRoute::FixPr), + "an unready packet must veto" + ); + } + + #[test] + fn pull_request_lifecycle_parses_each_decision() { + for (decision, expected) in [ + ("runnable_successor", NextStep::RunnableSuccessor), + ("monitor_continuation", NextStep::MonitorContinuation), + ("user_gate", NextStep::UserGate), + ("no_followup", NextStep::NoFollowup), + ] { + // The real packet shape: state sits under `observation` and the bucket + // under `grouped_monitor_projection`, verified against the CLI. + let packet = serde_json::json!({ + "observation": {"state": "OPEN"}, + "grouped_monitor_projection": {"state_bucket": "review_required"}, + "transition": { + "decision": decision, + "reason": "a compact reason", + "required_write_scopes": ["write"], + }, + }); + let outcome = parse_pull_request(&packet).expect("packet parses"); + assert_eq!(outcome.next_step, expected, "for {decision}"); + assert_eq!(outcome.state, "OPEN"); + assert_eq!(outcome.state_bucket, "review_required"); + assert_eq!(outcome.required_write_scopes, vec!["write"]); + } + } + + #[test] + fn an_unrecognized_lifecycle_decision_is_an_error() { + let packet = serde_json::json!({"transition": {"decision": "merge_it_now"}}); + let error = parse_pull_request(&packet).expect_err("unknown decisions are rejected"); + assert!(matches!( + error, + OrchestratorError::UnexpectedPacket { + field: "transition.decision", + .. + } + )); + } + + #[test] + fn missing_optional_fields_fall_back_rather_than_failing() { + // Optional evidence should degrade to empty, unlike the decision fields + // above, where guessing would be unsafe. + let packet = serde_json::json!({ + "caller_repo_branch": { + "issue_branch": "b", + "base_branch": "main", + "branch_action": "dry_run", + }, + "review_packet": {}, + }); + let outcome = parse_branch(&packet).expect("packet parses"); + assert!(!outcome.branch_ready); + assert!(outcome.changed_files.is_empty()); + assert!(outcome.review_packet_summary.is_empty()); + } + + #[cfg(windows)] + #[test] + fn windows_validation_commands_are_delegated_through_cmd() { + assert_eq!( + windows_safe_validation_command("pnpm test"), + "cmd /c pnpm test" + ); + assert_eq!( + windows_safe_validation_command("pnpm --dir src/web-ui run test:run x"), + "cmd /c pnpm --dir src/web-ui run test:run x" + ); + assert_eq!( + windows_safe_validation_command("cmd /c pnpm test"), + "cmd /c pnpm test" + ); + assert_eq!( + windows_safe_validation_command("cmd.exe /c pnpm test"), + "cmd.exe /c pnpm test" + ); + assert_eq!( + windows_safe_validation_command(" cmd /c pnpm test"), + " cmd /c pnpm test" + ); + } + + #[cfg(not(windows))] + #[test] + fn non_windows_validation_commands_are_passed_through() { + assert_eq!(windows_safe_validation_command("pnpm test"), "pnpm test"); + assert_eq!( + windows_safe_validation_command("cmd /c pnpm test"), + "cmd /c pnpm test" + ); + } +} diff --git a/src/crates/services/services-integrations/src/loopx_issue_fix/repository_context.rs b/src/crates/services/services-integrations/src/loopx_issue_fix/repository_context.rs new file mode 100644 index 0000000000..598a7ac987 --- /dev/null +++ b/src/crates/services/services-integrations/src/loopx_issue_fix/repository_context.rs @@ -0,0 +1,934 @@ +//! Build LoopX's `issue_fix_repository_context_input_v0` payload. +//! +//! This is the evidence half of the integration: LoopX holds no code-reading +//! ability and refuses to guess, so the quality of its route decisions depends +//! entirely on what this module reports. Its validator is strict, and a rejected +//! payload costs a whole subprocess round trip — so every constraint LoopX +//! enforces is enforced here too, at construction time. +//! +//! The rule that matters most: LoopX treats an aspect as *grounded* only when a +//! source is `freshness: current`, has `trust` of `authoritative` or `verified`, +//! and is not an external expert. It reports the whole context as grounded only +//! when `change_scope`, `reproduction`, and `validation` are all grounded. +//! +//! Grounding is not by itself the PR gate, though. Testing against the real CLI +//! showed that `--validation-label` — "how will you check this fix" — is what +//! actually permits the `fix_pr` route; a merely partial context still allows it, +//! and a fully grounded one without that label does not. Context grounding shapes +//! LoopX's reason codes and tells a caller what is still worth reading. + +use std::collections::BTreeSet; +use std::fmt; + +use serde::{Deserialize, Serialize}; + +pub const SCHEMA_VERSION: &str = "issue_fix_repository_context_input_v0"; + +/// LoopX rejects a payload with more than this many sources. +pub const MAX_SOURCES: usize = 16; + +const MAX_SOURCE_ID_CHARS: usize = 120; +const MAX_REFERENCE_CHARS: usize = 260; +const MAX_SUMMARY_CHARS: usize = 220; + +/// Where a piece of evidence came from. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum SourceKind { + RepositoryPolicy, + ArchitectureDoc, + MaintainerMap, + TestSurface, + SourceCode, + PriorFix, + /// LoopX requires `Advisory` trust for this kind. + MemoryRetrieval, + /// LoopX requires `Advisory` trust for this kind, and never counts it as + /// grounding an aspect. + ExternalExpert, + KnowledgeBundle, +} + +/// How much weight LoopX may place on a source. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum Trust { + Authoritative, + Verified, + Advisory, +} + +/// Whether a source was read at the pinned revision. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum Freshness { + /// Requires a `repository_revision` on the context. + Current, + Stale, + Unknown, +} + +/// Which question a source helps answer. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum SupportAspect { + Architecture, + Ownership, + ChangeScope, + Reproduction, + Validation, +} + +impl SupportAspect { + /// The three aspects LoopX weighs when classifying a context's grounding. + pub const REQUIRED_FOR_FIX: [Self; 3] = + [Self::ChangeScope, Self::Reproduction, Self::Validation]; +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum RepositoryContextError { + EmptyField { + field: &'static str, + }, + TooLong { + field: &'static str, + limit: usize, + actual: usize, + }, + InvalidSourceId { + source_id: String, + }, + AbsoluteReference { + reference: String, + }, + TraversingReference { + reference: String, + }, + InvalidReferenceUrl { + reference: String, + reason: &'static str, + }, + NoSupportedAspects { + source_id: String, + }, + TrustMustBeAdvisory { + source_id: String, + }, + CurrentFreshnessNeedsRevision { + source_id: String, + }, + DuplicateSourceId { + source_id: String, + }, + TooManySources { + limit: usize, + actual: usize, + }, + NoSources, +} + +impl fmt::Display for RepositoryContextError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::EmptyField { field } => { + write!(f, "{field} must not be empty") + } + Self::TooLong { + field, + limit, + actual, + } => write!( + f, + "{field} is {actual} characters, exceeding LoopX's limit of {limit}" + ), + Self::InvalidSourceId { source_id } => write!( + f, + "source id {source_id:?} must start alphanumeric and use only letters, digits, '_', '.', ':', or '-'" + ), + Self::AbsoluteReference { reference } => write!( + f, + "reference {reference:?} must be repository-relative; LoopX rejects absolute and home-relative paths as unsafe to publish" + ), + Self::TraversingReference { reference } => write!( + f, + "reference {reference:?} must not traverse outside the repository" + ), + Self::InvalidReferenceUrl { reference, reason } => { + write!(f, "reference URL {reference:?} {reason}") + } + Self::NoSupportedAspects { source_id } => write!( + f, + "source {source_id:?} must support at least one aspect" + ), + Self::TrustMustBeAdvisory { source_id } => write!( + f, + "source {source_id:?} is a memory retrieval or external expert, which LoopX requires to be advisory" + ), + Self::CurrentFreshnessNeedsRevision { source_id } => write!( + f, + "source {source_id:?} claims current freshness, which requires a repository revision" + ), + Self::DuplicateSourceId { source_id } => { + write!(f, "source id {source_id:?} appears more than once") + } + Self::TooManySources { limit, actual } => { + write!(f, "{actual} sources exceeds LoopX's limit of {limit}") + } + Self::NoSources => write!(f, "a repository context needs at least one source"), + } + } +} + +impl std::error::Error for RepositoryContextError {} + +/// One validated piece of evidence. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct RepositoryContextSource { + pub source_id: String, + pub source_kind: SourceKind, + pub reference: String, + pub trust: Trust, + pub freshness: Freshness, + pub supports: Vec, + pub summary: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub consultation_state: Option, +} + +/// A validated context payload, ready to serialize for LoopX. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct RepositoryContext { + pub schema_version: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub repository_revision: Option, + pub sources: Vec, +} + +/// How LoopX will classify one aspect's coverage. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum AspectStatus { + /// A current, trusted, non-expert source covers it. + Grounded, + /// Only weaker sources cover it. + Advisory, + Missing, +} + +/// What LoopX will report for the context as a whole. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ContextStatus { + /// All three fix-required aspects are grounded. + Grounded, + Partial, + Ungrounded, +} + +/// Accumulates sources and validates each as it is added. +/// +/// Validating on `push` rather than at build time means a caller learns which +/// source is wrong, instead of getting one failure for the whole payload. +#[derive(Debug, Clone, Default)] +pub struct RepositoryContextBuilder { + repository_revision: Option, + sources: Vec, +} + +impl RepositoryContextBuilder { + pub fn new() -> Self { + Self::default() + } + + /// Pin the revision the sources were read at. + /// + /// Required before any source may claim `Freshness::Current`, which in turn + /// is required for that source to ground an aspect. + pub fn repository_revision(mut self, revision: impl Into) -> Self { + let revision = revision.into(); + self.repository_revision = (!revision.trim().is_empty()).then_some(revision); + self + } + + pub fn has_revision(&self) -> bool { + self.repository_revision.is_some() + } + + /// Validate and append one source. + pub fn push( + &mut self, + source: RepositoryContextSource, + ) -> Result<&mut Self, RepositoryContextError> { + let source = self.validate(source)?; + self.sources.push(source); + Ok(self) + } + + fn validate( + &self, + mut source: RepositoryContextSource, + ) -> Result { + source.source_id = validate_source_id(&source.source_id)?; + source.reference = validate_reference(&source.reference)?; + source.summary = validate_text(&source.summary, "summary", MAX_SUMMARY_CHARS)?; + + if source.supports.is_empty() { + return Err(RepositoryContextError::NoSupportedAspects { + source_id: source.source_id, + }); + } + // LoopX sorts and dedupes these; matching here keeps the payload stable. + source.supports = source + .supports + .iter() + .copied() + .collect::>() + .into_iter() + .collect(); + + if matches!( + source.source_kind, + SourceKind::MemoryRetrieval | SourceKind::ExternalExpert + ) && source.trust != Trust::Advisory + { + return Err(RepositoryContextError::TrustMustBeAdvisory { + source_id: source.source_id, + }); + } + + if source.freshness == Freshness::Current && !self.has_revision() { + return Err(RepositoryContextError::CurrentFreshnessNeedsRevision { + source_id: source.source_id, + }); + } + + if self + .sources + .iter() + .any(|existing| existing.source_id == source.source_id) + { + return Err(RepositoryContextError::DuplicateSourceId { + source_id: source.source_id, + }); + } + + if self.sources.len() + 1 > MAX_SOURCES { + return Err(RepositoryContextError::TooManySources { + limit: MAX_SOURCES, + actual: self.sources.len() + 1, + }); + } + + Ok(source) + } + + /// Classify one aspect exactly as LoopX will. + pub fn aspect_status(&self, aspect: SupportAspect) -> AspectStatus { + let matching = self + .sources + .iter() + .filter(|source| source.supports.contains(&aspect)); + let mut any_match = false; + for source in matching { + any_match = true; + if source.freshness == Freshness::Current + && matches!(source.trust, Trust::Authoritative | Trust::Verified) + && source.source_kind != SourceKind::ExternalExpert + { + return AspectStatus::Grounded; + } + } + if any_match { + AspectStatus::Advisory + } else { + AspectStatus::Missing + } + } + + /// Predict LoopX's overall verdict without spending a subprocess call. + pub fn context_status(&self) -> ContextStatus { + let statuses = SupportAspect::REQUIRED_FOR_FIX.map(|aspect| self.aspect_status(aspect)); + if statuses.iter().all(|s| *s == AspectStatus::Grounded) { + ContextStatus::Grounded + } else if statuses.contains(&AspectStatus::Grounded) { + ContextStatus::Partial + } else { + ContextStatus::Ungrounded + } + } + + /// Which fix-required aspects are not yet grounded. + /// + /// A caller uses this to decide what else to read before asking LoopX. Gaps + /// here weaken the context rather than block a fix outright, so treat this as + /// a reading list, not a hard gate. + pub fn ungrounded_required_aspects(&self) -> Vec { + SupportAspect::REQUIRED_FOR_FIX + .into_iter() + .filter(|aspect| self.aspect_status(*aspect) != AspectStatus::Grounded) + .collect() + } + + pub fn build(self) -> Result { + if self.sources.is_empty() { + return Err(RepositoryContextError::NoSources); + } + Ok(RepositoryContext { + schema_version: SCHEMA_VERSION.to_string(), + repository_revision: self.repository_revision, + sources: self.sources, + }) + } +} + +fn validate_text( + value: &str, + field: &'static str, + limit: usize, +) -> Result { + // LoopX collapses whitespace before measuring, so do the same or a payload + // that looks short enough here could still be rejected there. + let compact = value.split_whitespace().collect::>().join(" "); + if compact.is_empty() { + return Err(RepositoryContextError::EmptyField { field }); + } + let actual = compact.chars().count(); + if actual > limit { + return Err(RepositoryContextError::TooLong { + field, + limit, + actual, + }); + } + Ok(compact) +} + +fn validate_source_id(value: &str) -> Result { + let id = validate_text(value, "source_id", MAX_SOURCE_ID_CHARS)?; + let mut chars = id.chars(); + let valid = chars + .next() + .is_some_and(|first| first.is_ascii_alphanumeric()) + && chars.all(|c| c.is_ascii_alphanumeric() || matches!(c, '_' | '.' | ':' | '-')); + if !valid { + return Err(RepositoryContextError::InvalidSourceId { source_id: id }); + } + Ok(id) +} + +/// Enforce LoopX's publish-safety rules on a reference. +/// +/// A local absolute path would leak the operator's filesystem layout into a +/// payload that may reach a public issue thread, so LoopX rejects it — and so +/// does this, before the round trip. +fn validate_reference(value: &str) -> Result { + let reference = validate_text(value, "reference", MAX_REFERENCE_CHARS)?; + + if reference.contains("://") { + return validate_reference_url(reference); + } + + if reference.starts_with('/') || reference.starts_with('~') || is_windows_absolute(&reference) { + return Err(RepositoryContextError::AbsoluteReference { reference }); + } + if reference.split(['/', '\\']).any(|segment| segment == "..") { + return Err(RepositoryContextError::TraversingReference { reference }); + } + // LoopX parses references as POSIX paths, so normalize separators rather than + // sending a Windows-style path it would treat as one long segment. + Ok(reference.replace('\\', "/")) +} + +fn validate_reference_url(reference: String) -> Result { + let Some((scheme, rest)) = reference.split_once("://") else { + return Err(RepositoryContextError::InvalidReferenceUrl { + reference, + reason: "must be a well-formed URL", + }); + }; + if scheme != "https" { + return Err(RepositoryContextError::InvalidReferenceUrl { + reference, + reason: "must use https", + }); + } + let authority = rest.split(['/', '?', '#']).next().unwrap_or_default(); + if authority.is_empty() { + return Err(RepositoryContextError::InvalidReferenceUrl { + reference, + reason: "must name a host", + }); + } + if authority.contains('@') { + return Err(RepositoryContextError::InvalidReferenceUrl { + reference, + reason: "must not embed user info", + }); + } + if rest.contains('?') { + return Err(RepositoryContextError::InvalidReferenceUrl { + reference, + reason: "must not contain query parameters", + }); + } + Ok(reference) +} + +fn is_windows_absolute(value: &str) -> bool { + let bytes = value.as_bytes(); + bytes.len() >= 3 + && bytes[0].is_ascii_alphabetic() + && bytes[1] == b':' + && matches!(bytes[2], b'/' | b'\\') +} + +#[cfg(test)] +mod tests { + use super::*; + + fn source( + id: &str, + kind: SourceKind, + trust: Trust, + freshness: Freshness, + supports: &[SupportAspect], + ) -> RepositoryContextSource { + RepositoryContextSource { + source_id: id.to_string(), + source_kind: kind, + reference: "src/lib.rs".to_string(), + trust, + freshness, + supports: supports.to_vec(), + summary: "a compact public-safe summary".to_string(), + consultation_state: None, + } + } + + fn grounded_builder() -> RepositoryContextBuilder { + let mut builder = RepositoryContextBuilder::new().repository_revision("abc123"); + builder + .push(source( + "scope", + SourceKind::SourceCode, + Trust::Verified, + Freshness::Current, + &[SupportAspect::ChangeScope, SupportAspect::Reproduction], + )) + .expect("scope source is valid"); + builder + .push(source( + "validation", + SourceKind::TestSurface, + Trust::Verified, + Freshness::Current, + &[SupportAspect::Validation], + )) + .expect("validation source is valid"); + builder + } + + #[test] + fn a_context_covering_all_required_aspects_is_grounded() { + let builder = grounded_builder(); + assert_eq!(builder.context_status(), ContextStatus::Grounded); + assert!(builder.ungrounded_required_aspects().is_empty()); + } + + #[test] + fn a_missing_validation_source_leaves_the_context_partial() { + // Verified against the real CLI: LoopX reports this exact shape as + // `context_status: partial` with validation as the sole unresolved aspect. + let mut builder = RepositoryContextBuilder::new().repository_revision("abc123"); + builder + .push(source( + "scope", + SourceKind::SourceCode, + Trust::Verified, + Freshness::Current, + &[SupportAspect::ChangeScope, SupportAspect::Reproduction], + )) + .expect("scope source is valid"); + + assert_eq!(builder.context_status(), ContextStatus::Partial); + assert_eq!( + builder.ungrounded_required_aspects(), + vec![SupportAspect::Validation] + ); + } + + #[test] + fn stale_sources_ground_nothing() { + let mut builder = RepositoryContextBuilder::new().repository_revision("abc123"); + builder + .push(source( + "stale", + SourceKind::SourceCode, + Trust::Verified, + Freshness::Stale, + &SupportAspect::REQUIRED_FOR_FIX, + )) + .expect("stale source is still valid"); + + assert_eq!(builder.context_status(), ContextStatus::Ungrounded); + assert_eq!( + builder.aspect_status(SupportAspect::ChangeScope), + AspectStatus::Advisory + ); + } + + #[test] + fn advisory_trust_grounds_nothing() { + let mut builder = RepositoryContextBuilder::new().repository_revision("abc123"); + builder + .push(source( + "memory", + SourceKind::MemoryRetrieval, + Trust::Advisory, + Freshness::Current, + &SupportAspect::REQUIRED_FOR_FIX, + )) + .expect("advisory memory source is valid"); + + assert_eq!(builder.context_status(), ContextStatus::Ungrounded); + } + + #[test] + fn an_external_expert_never_grounds_an_aspect() { + // LoopX excludes experts from grounding even when everything else lines + // up, because their answers still need local verification. + let mut builder = RepositoryContextBuilder::new().repository_revision("abc123"); + builder + .push(source( + "expert", + SourceKind::ExternalExpert, + Trust::Advisory, + Freshness::Current, + &SupportAspect::REQUIRED_FOR_FIX, + )) + .expect("expert source is valid"); + + assert_eq!(builder.context_status(), ContextStatus::Ungrounded); + assert_eq!( + builder.aspect_status(SupportAspect::Validation), + AspectStatus::Advisory + ); + } + + #[test] + fn an_unmatched_aspect_is_missing_not_advisory() { + let builder = RepositoryContextBuilder::new().repository_revision("abc123"); + assert_eq!( + builder.aspect_status(SupportAspect::Ownership), + AspectStatus::Missing + ); + } + + #[test] + fn memory_retrieval_must_be_advisory() { + let mut builder = RepositoryContextBuilder::new().repository_revision("abc123"); + let error = builder + .push(source( + "memory", + SourceKind::MemoryRetrieval, + Trust::Verified, + Freshness::Current, + &[SupportAspect::Architecture], + )) + .expect_err("verified memory retrieval is rejected"); + assert!(matches!( + error, + RepositoryContextError::TrustMustBeAdvisory { .. } + )); + } + + #[test] + fn current_freshness_requires_a_revision() { + let mut builder = RepositoryContextBuilder::new(); + let error = builder + .push(source( + "scope", + SourceKind::SourceCode, + Trust::Verified, + Freshness::Current, + &[SupportAspect::ChangeScope], + )) + .expect_err("current freshness without a revision is rejected"); + assert!(matches!( + error, + RepositoryContextError::CurrentFreshnessNeedsRevision { .. } + )); + } + + #[test] + fn a_blank_revision_does_not_count() { + let builder = RepositoryContextBuilder::new().repository_revision(" "); + assert!(!builder.has_revision()); + } + + #[test] + fn absolute_references_are_rejected() { + // The whole point: a local path would leak the operator's filesystem into + // a payload that can reach a public thread. + for path in [ + "/home/user/repo/src/lib.rs", + "~/repo/src/lib.rs", + "C:/codeagent/BitFun/src/lib.rs", + "C:\\codeagent\\BitFun\\src\\lib.rs", + ] { + let error = validate_reference(path).expect_err("absolute paths are rejected"); + assert!( + matches!(error, RepositoryContextError::AbsoluteReference { .. }), + "{path} produced {error:?}" + ); + } + } + + #[test] + fn traversing_references_are_rejected() { + for path in ["../secrets.txt", "src/../../etc/passwd", "src\\..\\out.txt"] { + let error = validate_reference(path).expect_err("traversal is rejected"); + assert!( + matches!(error, RepositoryContextError::TraversingReference { .. }), + "{path} produced {error:?}" + ); + } + } + + #[test] + fn windows_separators_are_normalized_to_posix() { + // LoopX parses references as POSIX paths, so a backslash path would look + // like one long segment to it. + let reference = + validate_reference("src\\web-ui\\src\\app.tsx").expect("relative path is accepted"); + assert_eq!(reference, "src/web-ui/src/app.tsx"); + } + + #[test] + fn a_bare_drive_letter_is_not_treated_as_absolute() { + // "C:" without a separator is a valid relative name, not a drive root. + assert!(validate_reference("C:file.rs").is_ok()); + } + + #[test] + fn https_urls_are_accepted_without_query_parameters() { + let reference = validate_reference("https://github.com/example/repo/blob/main/README.md") + .expect("plain https URL is accepted"); + assert!(reference.starts_with("https://")); + } + + #[test] + fn unsafe_urls_are_rejected() { + for (url, expected) in [ + ("http://example.com/a", "must use https"), + ("https://user:pw@example.com/a", "must not embed user info"), + ( + "https://example.com/a?token=secret", + "must not contain query parameters", + ), + ("https:///no-host", "must name a host"), + ] { + let error = validate_reference(url).expect_err("unsafe URL is rejected"); + match error { + RepositoryContextError::InvalidReferenceUrl { reason, .. } => { + assert_eq!(reason, expected, "for {url}"); + } + other => panic!("expected a URL error for {url}, got {other:?}"), + } + } + } + + #[test] + fn source_ids_must_match_loopx_shape() { + let mut builder = RepositoryContextBuilder::new(); + for bad in ["_leading", "-leading", "has space", "has/slash"] { + let mut candidate = source( + bad, + SourceKind::SourceCode, + Trust::Verified, + Freshness::Unknown, + &[SupportAspect::ChangeScope], + ); + candidate.source_id = bad.to_string(); + let error = builder.push(candidate).expect_err("invalid id is rejected"); + assert!( + matches!(error, RepositoryContextError::InvalidSourceId { .. }), + "{bad} produced {error:?}" + ); + } + // The permitted punctuation still works. + assert!(builder + .push(source( + "bitfun.workspace:icon-branch_1", + SourceKind::SourceCode, + Trust::Verified, + Freshness::Unknown, + &[SupportAspect::ChangeScope], + )) + .is_ok()); + } + + #[test] + fn duplicate_source_ids_are_rejected() { + let mut builder = grounded_builder(); + let error = builder + .push(source( + "scope", + SourceKind::SourceCode, + Trust::Verified, + Freshness::Current, + &[SupportAspect::Architecture], + )) + .expect_err("a repeated id is rejected"); + assert!(matches!( + error, + RepositoryContextError::DuplicateSourceId { .. } + )); + } + + #[test] + fn the_source_limit_is_enforced() { + let mut builder = RepositoryContextBuilder::new(); + for index in 0..MAX_SOURCES { + builder + .push(source( + &format!("source{index}"), + SourceKind::SourceCode, + Trust::Verified, + Freshness::Unknown, + &[SupportAspect::ChangeScope], + )) + .expect("sources within the limit are accepted"); + } + let error = builder + .push(source( + "overflow", + SourceKind::SourceCode, + Trust::Verified, + Freshness::Unknown, + &[SupportAspect::ChangeScope], + )) + .expect_err("one source past the limit is rejected"); + assert!(matches!( + error, + RepositoryContextError::TooManySources { + limit: MAX_SOURCES, + actual: 17 + } + )); + } + + #[test] + fn summaries_are_bounded_and_whitespace_collapsed() { + let mut builder = RepositoryContextBuilder::new(); + let mut candidate = source( + "long", + SourceKind::SourceCode, + Trust::Verified, + Freshness::Unknown, + &[SupportAspect::ChangeScope], + ); + candidate.summary = "s".repeat(MAX_SUMMARY_CHARS + 1); + let error = builder + .push(candidate) + .expect_err("an oversized summary is rejected"); + assert!(matches!( + error, + RepositoryContextError::TooLong { + field: "summary", + .. + } + )); + + let mut spaced = source( + "spaced", + SourceKind::SourceCode, + Trust::Verified, + Freshness::Unknown, + &[SupportAspect::ChangeScope], + ); + spaced.summary = " collapse these\n\nspaces ".to_string(); + builder.push(spaced).expect("whitespace is collapsed"); + assert_eq!(builder.sources[0].summary, "collapse these spaces"); + } + + #[test] + fn a_source_needs_at_least_one_aspect() { + let mut builder = RepositoryContextBuilder::new(); + let error = builder + .push(source( + "empty", + SourceKind::SourceCode, + Trust::Verified, + Freshness::Unknown, + &[], + )) + .expect_err("a source with no aspects is rejected"); + assert!(matches!( + error, + RepositoryContextError::NoSupportedAspects { .. } + )); + } + + #[test] + fn supports_are_sorted_and_deduplicated() { + let mut builder = RepositoryContextBuilder::new(); + builder + .push(source( + "dupes", + SourceKind::SourceCode, + Trust::Verified, + Freshness::Unknown, + &[ + SupportAspect::Validation, + SupportAspect::Architecture, + SupportAspect::Validation, + ], + )) + .expect("duplicate aspects are tolerated"); + assert_eq!( + builder.sources[0].supports, + vec![SupportAspect::Architecture, SupportAspect::Validation] + ); + } + + #[test] + fn an_empty_context_cannot_be_built() { + let error = RepositoryContextBuilder::new() + .build() + .expect_err("an empty context is rejected"); + assert_eq!(error, RepositoryContextError::NoSources); + } + + #[test] + fn the_payload_serializes_to_loopx_field_names() { + let context = grounded_builder().build().expect("context builds"); + let json = serde_json::to_value(&context).expect("context serializes"); + + assert_eq!(json["schema_version"], SCHEMA_VERSION); + assert_eq!(json["repository_revision"], "abc123"); + assert_eq!(json["sources"][0]["source_kind"], "source_code"); + assert_eq!(json["sources"][0]["trust"], "verified"); + assert_eq!(json["sources"][0]["freshness"], "current"); + assert_eq!(json["sources"][0]["supports"][0], "change_scope"); + assert_eq!(json["sources"][1]["source_kind"], "test_surface"); + // LoopX rejects unknown fields, so an absent consultation state must be + // omitted rather than serialized as null. + assert!(json["sources"][0].get("consultation_state").is_none()); + } + + #[test] + fn a_revisionless_payload_omits_the_revision_field() { + let mut builder = RepositoryContextBuilder::new(); + builder + .push(source( + "unknown", + SourceKind::SourceCode, + Trust::Verified, + Freshness::Unknown, + &[SupportAspect::ChangeScope], + )) + .expect("source is valid"); + let json = serde_json::to_value(builder.build().expect("context builds")) + .expect("context serializes"); + assert!(json.get("repository_revision").is_none()); + } +} diff --git a/src/crates/services/services-integrations/src/plugin_source.rs b/src/crates/services/services-integrations/src/plugin_source.rs index fa50a80c20..089732082f 100644 --- a/src/crates/services/services-integrations/src/plugin_source.rs +++ b/src/crates/services/services-integrations/src/plugin_source.rs @@ -3157,6 +3157,7 @@ mod tests { map_activation_store_error, map_load_store_error, native_path_identity, persist_trust_bytes_with_parent_sync, read_bounded_reader, read_scanned_file, replace_file_atomically, trust_file_identity, trust_store_issue_code, workspace_scope, + native_path_identity, ManagedPluginSourceError, ManagedPluginSourceService, OperationScanBudget, PluginPackageManifest, PluginPackageRoot, PluginPackageScope, PluginSourceDiscovery, PluginSourceIssue, PluginSourceIssueCode, PluginSourceStoreError, PluginTrustScope, diff --git a/src/crates/services/services-integrations/src/review_platform.rs b/src/crates/services/services-integrations/src/review_platform.rs index dcd87661cb..bb105a288b 100644 --- a/src/crates/services/services-integrations/src/review_platform.rs +++ b/src/crates/services/services-integrations/src/review_platform.rs @@ -42,6 +42,9 @@ const DEFAULT_ISSUE_PAGE: u32 = 1; const DEFAULT_ISSUE_PAGE_SIZE: u32 = 100; const MAX_ISSUE_PAGE_SIZE: u32 = 100; const MAX_ISSUE_RESPONSE_BYTES: usize = 2 * 1024 * 1024; +/// A list page carries no bodies, so it needs far less headroom than one issue's +/// full evidence — but titles and label sets across 100 rows still add up. +const MAX_ISSUE_LIST_RESPONSE_BYTES: usize = 4 * 1024 * 1024; const MAX_ISSUE_COMMENTS_RESPONSE_BYTES: usize = 8 * 1024 * 1024; const MAX_ISSUE_BODY_CHARS: usize = 128_000; const MAX_ISSUE_COMMENT_BODY_CHARS: usize = 32_000; @@ -322,6 +325,78 @@ pub struct ReviewPlatformIssueEvidence { pub next_cursor: Option, } +/// Inputs for [`ReviewPlatformService::list_issues`]. +#[derive(Debug, Clone, Copy)] +pub struct ReviewPlatformListIssuesRequest<'a> { + pub platform: ReviewPlatformKind, + pub host: &'a str, + pub project_path: &'a str, + pub state: ReviewPlatformIssueState, + pub page: Option, + pub per_page: Option, + /// Local checkout used to resolve provider auth, when one is available. + pub repository_path: Option<&'a str>, +} + +/// Which issues to enumerate. The two providers spell these differently. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ReviewPlatformIssueState { + #[default] + Open, + Closed, + All, +} + +impl ReviewPlatformIssueState { + fn github_value(self) -> &'static str { + match self { + Self::Open => "open", + Self::Closed => "closed", + Self::All => "all", + } + } + + /// `None` means "send no state filter", which is how GitLab expresses "all". + fn gitlab_value(self) -> Option<&'static str> { + match self { + Self::Open => Some("opened"), + Self::Closed => Some("closed"), + Self::All => None, + } + } +} + +/// One row of an issue list. +/// +/// Deliberately lighter than [`ReviewPlatformIssueEvidence`]: enumerating a +/// repository's open issues must not pull every body and comment thread. Callers +/// that need the full evidence for one issue fetch it separately. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ReviewPlatformIssueSummary { + pub issue_id: String, + pub number: i64, + pub title: String, + pub state: String, + pub author: Option, + pub labels: Vec, + pub comments_count: i64, + pub created_at: Option, + pub updated_at: Option, + pub web_url: String, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ReviewPlatformIssuePage { + pub platform: ReviewPlatformKind, + pub host: String, + pub project_path: String, + pub items: Vec, + pub pagination: ReviewPlatformPagination, +} + #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct ReviewPlatformCommit { @@ -1098,6 +1173,35 @@ impl ReviewPlatformService { acquire_issue_evidence(&context, &identity, IssuePagination::new(page, per_page)).await } + /// Enumerate a repository's issues, newest activity first. + /// + /// Pull requests are excluded even on GitHub, whose issues endpoint returns + /// them inline. Takes a request struct rather than positional parameters + /// because the sibling `issue` method is already at clippy's argument limit. + pub async fn list_issues( + &self, + request: ReviewPlatformListIssuesRequest<'_>, + ) -> Result { + let auth_tokens = self.load_stored_tokens().await?; + let host = normalize_provider_host(request.host)?; + let project_path = normalize_project_path(request.platform, request.project_path)?; + let context = self + .provider_context_for_identity_request( + request.platform, + &host, + &project_path, + request.repository_path, + &auth_tokens, + ) + .await?; + acquire_issue_page( + &context, + request.state, + IssuePagination::new(request.page, request.per_page), + ) + .await + } + pub async fn pull_request_review_target_by_identity( &self, platform: ReviewPlatformKind, @@ -3957,6 +4061,108 @@ async fn acquire_issue_evidence( } } +/// Enumerate a repository's issues. +/// +/// GitHub reaches the API through the `gh` CLI while GitLab uses HTTP, mirroring +/// [`acquire_issue_evidence`]. Both paths return summary rows only; a caller that +/// needs one issue's body and comments fetches it separately. +async fn acquire_issue_page( + context: &ProviderContext, + state: ReviewPlatformIssueState, + pagination: IssuePagination, +) -> Result { + let page = pagination.page.to_string(); + let per_page = pagination.per_page.to_string(); + let host = context.remote.host.clone(); + let project_path = context.remote.project_path.clone(); + + match context.remote.platform { + ReviewPlatformKind::Github => { + let url = format!( + "{}/repos/{}/{}/issues", + context.api_base_url, context.remote.owner, context.remote.repository_name + ); + let response = github_api_get_json( + context, + &url, + &[ + ("state".to_string(), state.github_value().to_string()), + ("page".to_string(), page), + ("per_page".to_string(), per_page), + ], + MAX_ISSUE_LIST_RESPONSE_BYTES, + ) + .await + .map_err(|error| review_evidence_error(error, "issue_list_response"))?; + + let raw = array_items(&response); + // `gh` gives no Link headers here, so infer another page from a full + // one. Pull requests are filtered out *after* that check: a page that + // is all PRs still means more issues may follow. + let has_next = raw.len() == pagination.per_page as usize; + let items = raw + .iter() + .filter_map(|issue| github_issue_summary_from_value(&host, &project_path, issue)) + .collect::>(); + + Ok(ReviewPlatformIssuePage { + platform: ReviewPlatformKind::Github, + host, + project_path, + items, + pagination: ReviewPlatformPagination { + page: pagination.page, + per_page: pagination.per_page, + total: None, + has_next, + }, + }) + } + ReviewPlatformKind::Gitlab => { + let project = urlencoding::encode(&project_path); + let url = format!("{}/projects/{}/issues", context.api_base_url, project); + let client = http_client()?; + let mut request = gitlab_request(client, &url, context.token.as_deref()).query(&[ + ("page", page.as_str()), + ("per_page", per_page.as_str()), + ("order_by", "updated_at"), + ("sort", "desc"), + ]); + // GitLab has no "all" literal — you omit the filter entirely. Sending + // `state=` would be a malformed value rather than an absent one. + if let Some(state) = state.gitlab_value() { + request = request.query(&[("state", state)]); + } + let response = + send_review_json_response_bounded(request, MAX_ISSUE_LIST_RESPONSE_BYTES) + .await + .map_err(|error| review_evidence_http_error(error, "issue_list_response"))?; + + let items = array_items(&response.value) + .iter() + .filter_map(|issue| gitlab_issue_summary_from_value(&host, &project_path, issue)) + .collect::>(); + + Ok(ReviewPlatformIssuePage { + platform: ReviewPlatformKind::Gitlab, + host, + project_path, + items, + pagination: ReviewPlatformPagination { + page: pagination.page, + per_page: pagination.per_page, + total: None, + // GitLab is authoritative about the next page via a header. + has_next: gitlab_next_page(&response.headers, pagination.page).is_some(), + }, + }) + } + platform => Err(ReviewPlatformError::UnsupportedPlatform( + platform_label(platform).to_string(), + )), + } +} + fn review_evidence_http_error(error: ReviewHttpError, resource: &str) -> ReviewPlatformError { match error { ReviewHttpError::ResponseTooLarge { limit_bytes } => { @@ -6193,6 +6399,84 @@ fn map_gitlab_issue( ) } +/// Map one GitHub issue list entry to a summary row. +/// +/// Returns `None` for pull requests: GitHub's issues endpoint returns them +/// alongside real issues, distinguished only by a `pull_request` member. Skipping +/// them here mirrors [`reject_pull_request_issue_target`] for the single-issue +/// path. +fn github_issue_summary_from_value( + host: &str, + project_path: &str, + issue: &Value, +) -> Option { + if issue.get("pull_request").is_some() { + return None; + } + let number = value_i64(issue, "number"); + if number <= 0 { + return None; + } + let labels = array_items(issue.get("labels").unwrap_or(&Value::Null)) + .iter() + .filter_map(|label| { + label + .as_str() + .map(str::to_string) + .or_else(|| optional_string(label, "name")) + }) + .collect::>(); + Some(ReviewPlatformIssueSummary { + issue_id: number.to_string(), + number, + title: value_string(issue, "title"), + state: value_string(issue, "state"), + author: nested_optional_string(issue, &["user", "login"]), + labels, + comments_count: value_i64(issue, "comments"), + created_at: optional_string(issue, "created_at"), + updated_at: optional_string(issue, "updated_at"), + web_url: first_non_empty(&[ + value_string(issue, "html_url"), + format!("https://{host}/{project_path}/issues/{number}"), + ]), + }) +} + +/// Map one GitLab issue list entry to a summary row. +/// +/// GitLab identifies issues by project-scoped `iid`, not the global `id`. +fn gitlab_issue_summary_from_value( + host: &str, + project_path: &str, + issue: &Value, +) -> Option { + let number = value_i64(issue, "iid"); + if number <= 0 { + return None; + } + let labels = array_items(issue.get("labels").unwrap_or(&Value::Null)) + .iter() + .filter_map(Value::as_str) + .map(str::to_string) + .collect::>(); + Some(ReviewPlatformIssueSummary { + issue_id: number.to_string(), + number, + title: value_string(issue, "title"), + state: value_string(issue, "state"), + author: nested_optional_string(issue, &["author", "username"]), + labels, + comments_count: value_i64(issue, "user_notes_count"), + created_at: optional_string(issue, "created_at"), + updated_at: optional_string(issue, "updated_at"), + web_url: first_non_empty(&[ + value_string(issue, "web_url"), + format!("https://{host}/{project_path}/-/issues/{number}"), + ]), + }) +} + #[allow(clippy::too_many_arguments)] fn finalize_issue_mapping( identity: &ProviderIssueIdentity, @@ -9768,4 +10052,238 @@ mod tests { provider_for(existing_remote_context.remote.platform), )); } + + #[test] + fn github_issue_summary_skips_pull_requests() { + // GitHub's issues endpoint returns PRs inline, marked only by this member. + // Enumerating issues must not surface them. + let pull_request = serde_json::json!({ + "number": 7, + "title": "a pull request", + "state": "open", + "pull_request": {"url": "https://api.github.com/repos/example/repo/pulls/7"}, + }); + assert!( + github_issue_summary_from_value("github.com", "example/repo", &pull_request).is_none() + ); + } + + #[test] + fn github_issue_summary_maps_labels_from_objects_and_strings() { + let issue = serde_json::json!({ + "number": 1849, + "title": "workspace icons are inconsistent", + "state": "open", + "comments": 2, + "html_url": "https://github.com/example/repo/issues/1849", + "user": {"login": "reporter"}, + "created_at": "2026-07-29T07:35:37Z", + "updated_at": "2026-07-30T06:16:48Z", + // GitHub sends label objects; some mirrors send bare strings. + "labels": [{"name": "bug"}, "needs-triage"], + }); + + let summary = github_issue_summary_from_value("github.com", "example/repo", &issue) + .expect("a real issue maps"); + + assert_eq!(summary.issue_id, "1849"); + assert_eq!(summary.number, 1849); + assert_eq!(summary.state, "open"); + assert_eq!(summary.author.as_deref(), Some("reporter")); + assert_eq!(summary.labels, vec!["bug", "needs-triage"]); + assert_eq!(summary.comments_count, 2); + assert_eq!( + summary.web_url, + "https://github.com/example/repo/issues/1849" + ); + } + + #[test] + fn github_issue_summary_falls_back_to_a_derived_url() { + let issue = serde_json::json!({"number": 5, "title": "no html_url", "state": "open"}); + let summary = github_issue_summary_from_value("github.example.com", "team/repo", &issue) + .expect("a real issue maps"); + assert_eq!( + summary.web_url, + "https://github.example.com/team/repo/issues/5" + ); + } + + #[test] + fn github_issue_summary_rejects_a_missing_number() { + let issue = serde_json::json!({"title": "no number", "state": "open"}); + assert!(github_issue_summary_from_value("github.com", "example/repo", &issue).is_none()); + } + + #[test] + fn gitlab_issue_summary_prefers_the_project_scoped_iid() { + // `id` is global and `iid` is project-scoped; only `iid` addresses the + // issue through the project API. + let issue = serde_json::json!({ + "id": 99001, + "iid": 12, + "title": "a gitlab issue", + "state": "opened", + "user_notes_count": 3, + "web_url": "https://gitlab.com/example/repo/-/issues/12", + "author": {"username": "reporter"}, + "labels": ["bug", "frontend"], + }); + + let summary = gitlab_issue_summary_from_value("gitlab.com", "example/repo", &issue) + .expect("a real issue maps"); + + assert_eq!(summary.issue_id, "12"); + assert_eq!(summary.number, 12); + assert_eq!(summary.comments_count, 3); + assert_eq!(summary.labels, vec!["bug", "frontend"]); + } + + #[test] + fn gitlab_issue_summary_rejects_a_missing_iid() { + // A global `id` alone is not addressable, so it must not pass. + let issue = serde_json::json!({"id": 99001, "title": "no iid", "state": "opened"}); + assert!(gitlab_issue_summary_from_value("gitlab.com", "example/repo", &issue).is_none()); + } + + #[test] + fn issue_state_maps_to_each_provider_vocabulary() { + assert_eq!(ReviewPlatformIssueState::Open.github_value(), "open"); + assert_eq!( + ReviewPlatformIssueState::Open.gitlab_value(), + Some("opened") + ); + assert_eq!(ReviewPlatformIssueState::Closed.github_value(), "closed"); + assert_eq!( + ReviewPlatformIssueState::Closed.gitlab_value(), + Some("closed") + ); + assert_eq!(ReviewPlatformIssueState::All.github_value(), "all"); + // GitLab has no "all" literal: the filter is omitted instead. + assert_eq!(ReviewPlatformIssueState::All.gitlab_value(), None); + assert_eq!( + ReviewPlatformIssueState::default(), + ReviewPlatformIssueState::Open + ); + } + + #[tokio::test] + async fn gitlab_issue_page_filters_pull_requests_and_reads_the_next_page_header() { + let body = serde_json::json!([ + { + "iid": 12, + "title": "first", + "state": "opened", + "user_notes_count": 1, + "web_url": "https://gitlab.com/example/repo/-/issues/12", + "author": {"username": "one"}, + "labels": ["bug"], + }, + // No iid: not addressable, so it must be dropped rather than mapped. + {"id": 99002, "title": "malformed", "state": "opened"}, + ]) + .to_string(); + let api_base_url = spawn_single_review_response( + format!( + "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nx-next-page: 2\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", + body.len() + ) + .into_bytes(), + ); + let context = gitlab_trace_context(api_base_url); + + let page = acquire_issue_page( + &context, + ReviewPlatformIssueState::Open, + IssuePagination::new(Some(1), Some(50)), + ) + .await + .expect("issue page should load"); + + assert_eq!(page.platform, ReviewPlatformKind::Gitlab); + assert_eq!(page.items.len(), 1); + assert_eq!(page.items[0].number, 12); + assert_eq!(page.pagination.per_page, 50); + // GitLab is authoritative about continuation via the header. + assert!(page.pagination.has_next); + } + + #[tokio::test] + async fn gitlab_issue_page_reports_no_next_page_without_the_header() { + let body = "[]"; + let api_base_url = spawn_single_review_response( + format!( + "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", + body.len() + ) + .into_bytes(), + ); + let context = gitlab_trace_context(api_base_url); + + let page = acquire_issue_page( + &context, + ReviewPlatformIssueState::All, + IssuePagination::new(None, None), + ) + .await + .expect("issue page should load"); + + assert!(page.items.is_empty()); + assert!(!page.pagination.has_next); + } + + #[tokio::test] + async fn issue_page_rejects_unsupported_platforms() { + let mut context = gitlab_trace_context("http://127.0.0.1:1".to_string()); + context.remote.platform = ReviewPlatformKind::Gitcode; + + let result = acquire_issue_page( + &context, + ReviewPlatformIssueState::Open, + IssuePagination::new(None, None), + ) + .await; + + assert!(matches!( + result, + Err(ReviewPlatformError::UnsupportedPlatform(_)) + )); + } + + /// Exercises the real GitHub path, which goes through the `gh` CLI rather than + /// HTTP — the mocked tests above cannot cover it. Ignored by default because it + /// needs network access and an authenticated `gh`. + #[tokio::test] + #[ignore = "requires network access and an authenticated gh CLI"] + async fn github_issue_page_enumerates_a_public_repository() { + let tokens = ReviewPlatformAuthTokens::default(); + let context = provider_context_for_identity( + ReviewPlatformKind::Github, + "github.com", + "GCWing/BitFun", + &tokens, + ) + .expect("public GitHub context should be valid"); + + let page = acquire_issue_page( + &context, + ReviewPlatformIssueState::Open, + IssuePagination::new(Some(1), Some(5)), + ) + .await + .expect("issue page should load from GitHub"); + + assert_eq!(page.platform, ReviewPlatformKind::Github); + assert_eq!(page.project_path, "GCWing/BitFun"); + assert!(!page.items.is_empty(), "the repository has open issues"); + for item in &page.items { + assert!(item.number > 0, "issue numbers are positive: {item:?}"); + assert!(!item.title.is_empty(), "issues have titles: {item:?}"); + assert_eq!(item.state, "open", "state filter applied: {item:?}"); + assert!( + item.web_url.contains("/issues/"), + "pull requests must be filtered out: {item:?}" + ); + } + } } diff --git a/src/crates/services/services-integrations/tests/loopx_issue_fix_contracts.rs b/src/crates/services/services-integrations/tests/loopx_issue_fix_contracts.rs new file mode 100644 index 0000000000..460b5da201 --- /dev/null +++ b/src/crates/services/services-integrations/tests/loopx_issue_fix_contracts.rs @@ -0,0 +1,616 @@ +//! Contract tests for the `loopx` issue-fix bridge. +//! +//! These exercise the real `loopx` CLI when it is installed. When it is not, each +//! test reports a skip rather than failing, so CI hosts without LoopX stay green — +//! the feature is probe-gated at runtime for exactly the same reason. + +use bitfun_services_integrations::loopx_issue_fix::orchestrator::{ + ContextGrounding, ExecutionMode, FixRoute, IssueFixOrchestrator, IssueFixRequest, NextStep, + ReproductionStatus, ScopeClass, +}; +use bitfun_services_integrations::loopx_issue_fix::repository_context::{ + ContextStatus, Freshness, RepositoryContextBuilder, RepositoryContextSource, SourceKind, + SupportAspect, Trust, +}; +use bitfun_services_integrations::loopx_issue_fix::{LoopxIssueFix, LoopxIssueFixError}; + +/// Resolve LoopX or explain the skip. Keeps the skip reason in one place. +fn loopx_or_skip(test_name: &str) -> Option { + match LoopxIssueFix::probe() { + Some(loopx) => Some(loopx), + None => { + eprintln!("skipping {test_name}: loopx is not installed on this host"); + None + } + } +} + +const ISSUE_URL: &str = "https://github.com/GCWing/BitFun/issues/1849"; + +/// A grounded repository context, built through the real generator. +/// +/// LoopX will not select `fix_pr` without one — an ungrounded request yields +/// `repository_context_not_provided` in its reason codes and falls back to +/// `triage_only`. Building this with `RepositoryContextBuilder` rather than a +/// hand-written literal is the point: it proves the generator's own prediction of +/// "grounded" matches what LoopX actually decides. +fn grounded_repository_context( +) -> bitfun_services_integrations::loopx_issue_fix::repository_context::RepositoryContext { + let mut builder = RepositoryContextBuilder::new() + .repository_revision("9ed5c5fec0000000000000000000000000000000"); + builder + .push(RepositoryContextSource { + source_id: "workspace-item-icon".to_string(), + source_kind: SourceKind::SourceCode, + reference: + "src/web-ui/src/app/components/NavPanel/sections/workspaces/WorkspaceItem.tsx" + .to_string(), + trust: Trust::Verified, + freshness: Freshness::Current, + supports: vec![ + SupportAspect::Architecture, + SupportAspect::ChangeScope, + SupportAspect::Reproduction, + ], + summary: "Icon ternary renders an arrow for the active workspace row and a folder for its siblings." + .to_string(), + consultation_state: None, + }) + .expect("the change-scope source is valid"); + builder + .push(RepositoryContextSource { + source_id: "workspace-layout-guard".to_string(), + source_kind: SourceKind::TestSurface, + reference: + "src/web-ui/src/app/components/NavPanel/sections/workspaces/WorkspaceListSectionLayout.test.ts" + .to_string(), + trust: Trust::Verified, + freshness: Freshness::Current, + supports: vec![SupportAspect::Validation], + summary: "Raw-text layout guard over the workspace component; focused coverage would need adding." + .to_string(), + consultation_state: None, + }) + .expect("the validation source is valid"); + + // If the generator and LoopX ever disagree about what grounds an aspect, this + // assertion fails before the subprocess call and localizes the bug here. + assert_eq!( + builder.context_status(), + ContextStatus::Grounded, + "the generator should predict a grounded context" + ); + + builder.build().expect("context builds") +} + +/// The same context, written to a temp file for the raw-CLI tests below. +fn write_repository_context(dir: &std::path::Path) -> std::path::PathBuf { + let context = grounded_repository_context(); + let path = dir.join("repository-context.json"); + std::fs::write( + &path, + serde_json::to_vec_pretty(&context).expect("context serializes"), + ) + .expect("context file is written"); + path +} + +/// Read-only feasibility flags. `--no-write-domain-state` keeps LoopX from +/// touching goal state, and every issue-fix projection is write-free by design. +fn feasibility_args<'a>(scope_class: &'a str, context_path: &'a str) -> Vec<&'a str> { + vec![ + "feasibility", + "--repo", + "GCWing/BitFun", + "--issue-ref", + "1849", + "--url", + ISSUE_URL, + "--reproduction-status", + "confirmed", + "--reproduction-label", + "workspace-row-icon-branch", + // Naming a validation surface is mandatory for `fix_pr`; without it LoopX + // reports `validation_surface_named` as unmet and downgrades to triage. + "--validation-label", + "web-ui focused vitest", + "--repository-context-json", + context_path, + "--no-write-domain-state", + "--scope-class", + scope_class, + ] +} + +#[test] +fn probe_reports_a_usable_program_path_when_loopx_is_installed() { + let Some(loopx) = loopx_or_skip("probe_reports_a_usable_program_path_when_loopx_is_installed") + else { + return; + }; + assert!( + loopx.program().is_file(), + "probe returned a path that is not a file: {}", + loopx.program().display() + ); +} + +#[tokio::test] +async fn bounded_scope_with_reproduction_selects_the_fix_pr_route() { + let Some(loopx) = loopx_or_skip("bounded_scope_with_reproduction_selects_the_fix_pr_route") + else { + return; + }; + let dir = tempfile::tempdir().expect("temp dir is created"); + let context = write_repository_context(dir.path()); + let context = context.to_str().expect("context path is valid UTF-8"); + + let packet = loopx + .issue_fix(feasibility_args("bounded", context)) + .await + .expect("feasibility projection succeeds"); + + assert_eq!(packet["decision"]["route"], "fix_pr"); + assert_eq!(packet["transition"]["decision"], "runnable_successor"); + // The whole integration rests on LoopX never writing; assert it explicitly. + assert_eq!(packet["external_writes_performed"], false); + assert_eq!(packet["todo_write_performed"], false); +} + +#[tokio::test] +async fn oversized_scope_refuses_to_open_a_pull_request() { + let Some(loopx) = loopx_or_skip("oversized_scope_refuses_to_open_a_pull_request") else { + return; + }; + let dir = tempfile::tempdir().expect("temp dir is created"); + let context = write_repository_context(dir.path()); + let context = context.to_str().expect("context path is valid UTF-8"); + + let packet = loopx + .issue_fix(feasibility_args("oversized", context)) + .await + .expect("feasibility projection succeeds"); + + // Evidence is present and the issue reproduces, yet an oversized change scope + // must still not produce a PR. This gate is the reason for the integration. + assert_eq!(packet["decision"]["route"], "triage_only"); + assert_eq!(packet["transition"]["decision"], "no_followup"); +} + +/// Naming a validation surface via `--validation-label` is mandatory for +/// `fix_pr`. A fully grounded context does not substitute for it: LoopX refuses to +/// open a PR when it cannot see how the fix would be checked. +/// +/// Read together with `the_generator_prediction_matches_what_loopx_decides`, which +/// shows the converse — the label without full grounding *is* enough. So the label +/// is the real gate, and context grounding is not. +#[tokio::test] +async fn omitting_the_validation_label_downgrades_to_triage() { + let Some(loopx) = loopx_or_skip("omitting_the_validation_label_downgrades_to_triage") else { + return; + }; + let dir = tempfile::tempdir().expect("temp dir is created"); + let context = write_repository_context(dir.path()); + let context = context.to_str().expect("context path is valid UTF-8"); + + let packet = loopx + .issue_fix([ + "feasibility", + "--repo", + "GCWing/BitFun", + "--issue-ref", + "1849", + "--url", + ISSUE_URL, + "--reproduction-status", + "confirmed", + "--reproduction-label", + "workspace-row-icon-branch", + "--repository-context-json", + context, + "--no-write-domain-state", + "--scope-class", + "bounded", + ]) + .await + .expect("feasibility projection succeeds"); + + assert_eq!(packet["decision"]["route"], "triage_only"); + let reasons = packet["decision"]["reason_codes"] + .as_array() + .expect("reason codes are an array"); + assert!( + reasons + .iter() + .any(|code| code == "repository_context_grounded"), + "grounding is intact; only the label is missing: {reasons:?}" + ); +} + +/// The generator predicts grounding locally so callers can decide what else to +/// read before paying for a subprocess call. That prediction is only useful if it +/// agrees with LoopX, so assert the agreement against the real CLI. +/// +/// Note what this does *not* claim: a partial context still permits `fix_pr` as +/// long as `--validation-label` names a validation surface. LoopX distinguishes +/// "which test files did you read" (a context source) from "how will you check +/// this fix" (the label), and only the latter gates the route. +#[tokio::test] +async fn the_generator_prediction_matches_what_loopx_decides() { + let Some(loopx) = loopx_or_skip("the_generator_prediction_matches_what_loopx_decides") else { + return; + }; + let dir = tempfile::tempdir().expect("temp dir is created"); + + // A context with no validation source: the generator must call this partial + // and name validation as the gap. + let mut builder = RepositoryContextBuilder::new() + .repository_revision("9ed5c5fec0000000000000000000000000000000"); + builder + .push(RepositoryContextSource { + source_id: "scope-only".to_string(), + source_kind: SourceKind::SourceCode, + reference: + "src/web-ui/src/app/components/NavPanel/sections/workspaces/WorkspaceItem.tsx" + .to_string(), + trust: Trust::Verified, + freshness: Freshness::Current, + supports: vec![SupportAspect::ChangeScope, SupportAspect::Reproduction], + summary: "Only change scope and reproduction; nothing covers validation.".to_string(), + consultation_state: None, + }) + .expect("the scope source is valid"); + + assert_eq!( + builder.context_status(), + ContextStatus::Partial, + "no validation source means partial" + ); + assert_eq!( + builder.ungrounded_required_aspects(), + vec![SupportAspect::Validation] + ); + + let path = dir.path().join("partial-context.json"); + std::fs::write( + &path, + serde_json::to_vec_pretty(&builder.build().expect("context builds")) + .expect("context serializes"), + ) + .expect("context file is written"); + let path = path.to_str().expect("path is valid UTF-8"); + + let packet = loopx + .issue_fix(feasibility_args("bounded", path)) + .await + .expect("feasibility projection succeeds"); + + // LoopX must reach the same verdict the generator predicted, aspect for + // aspect. This is the assertion that catches drift between the two. + let context = &packet["observation"]["repository_context"]; + assert_eq!(context["context_status"], "partial"); + assert_eq!( + context["unresolved_required_aspects"] + .as_array() + .expect("unresolved aspects are an array"), + &vec![serde_json::Value::from("validation")] + ); + assert_eq!(context["coverage"]["change_scope"]["status"], "grounded"); + assert_eq!(context["coverage"]["reproduction"]["status"], "grounded"); + assert_eq!(context["coverage"]["validation"]["status"], "missing"); + + let reasons = packet["decision"]["reason_codes"] + .as_array() + .expect("reason codes are an array") + .iter() + .filter_map(|code| code.as_str()) + .collect::>(); + assert!( + reasons.contains(&"repository_context_partial"), + "a partial context must be recorded as such: {reasons:?}" + ); + assert!( + !reasons.contains(&"repository_context_grounded"), + "a partial context must not read as grounded: {reasons:?}" + ); +} + +#[tokio::test] +async fn in_band_refusal_becomes_a_rejected_error() { + let Some(loopx) = loopx_or_skip("in_band_refusal_becomes_a_rejected_error") else { + return; + }; + + // Omitting --reproduction-label makes LoopX refuse. It reports refusals as + // `{"ok": false, "error": ...}` on stdout *and* exits nonzero, so the bridge + // must parse stdout first — otherwise the reason is lost behind a bare exit + // code, which is exactly the bug this test caught. + let error = loopx + .issue_fix([ + "feasibility", + "--repo", + "GCWing/BitFun", + "--issue-ref", + "1849", + "--url", + ISSUE_URL, + "--reproduction-status", + "confirmed", + "--scope-class", + "bounded", + "--no-write-domain-state", + ]) + .await + .expect_err("a missing required label must surface as an error"); + + match error { + LoopxIssueFixError::Rejected(reason) => { + assert!( + reason.contains("reproduction_label"), + "unexpected refusal reason: {reason}" + ); + } + other => panic!("expected an in-band refusal, got {other:?}"), + } +} + +#[tokio::test] +async fn an_unknown_subcommand_surfaces_a_nonzero_exit() { + let Some(loopx) = loopx_or_skip("an_unknown_subcommand_surfaces_a_nonzero_exit") else { + return; + }; + + let error = loopx + .issue_fix(["definitely-not-a-subcommand"]) + .await + .expect_err("an unknown subcommand must fail"); + + assert!( + matches!(error, LoopxIssueFixError::Exit { .. }), + "expected a nonzero exit, got {error:?}" + ); +} + +/// LoopX's own subprocess calls omit `encoding=`, so on a non-UTF-8 locale it +/// decodes `gh` output as the local codepage and dies. The bridge sets +/// `PYTHONUTF8=1` to fix all of its call sites at once; this test proves the +/// fetch path works, which is exactly what fails without it. +#[tokio::test] +async fn fetching_public_metadata_survives_a_non_utf8_host_locale() { + let Some(loopx) = loopx_or_skip("fetching_public_metadata_survives_a_non_utf8_host_locale") + else { + return; + }; + + let result = loopx + .issue_fix([ + "workflow-plan", + "--repo", + "GCWing/BitFun", + "--issue-ref", + "1849", + "--url", + ISSUE_URL, + "--fetch-metadata", + ]) + .await; + + match result { + Ok(packet) => { + assert_eq!(packet["external_reads_performed"], true); + // The issue title is Chinese; reaching this point means no mojibake. + assert_eq!(packet["issue_signal"]["repo"], "GCWing/BitFun"); + } + // `gh` may be absent or unauthenticated on a CI host. That is an + // environment gap, not an encoding regression, so tolerate it — but let + // any other failure fail the test. + Err(LoopxIssueFixError::Rejected(reason)) => { + assert!( + reason.contains("gh") || reason.contains("metadata fetch"), + "unexpected refusal while fetching metadata: {reason}" + ); + eprintln!("tolerating environment gap: {reason}"); + } + Err(other) => panic!("metadata fetch failed unexpectedly: {other:?}"), + } +} + +/// One issue's request, pointing at the real BitFun repository. +fn issue_request<'a>( + context: &'a bitfun_services_integrations::loopx_issue_fix::repository_context::RepositoryContext, + scope_class: ScopeClass, +) -> IssueFixRequest<'a> { + IssueFixRequest { + repo: "GCWing/BitFun", + issue_ref: "1849", + issue_url: ISSUE_URL, + context, + validation_label: "web-ui focused vitest", + reproduction_label: "workspace-row-icon-branch", + reproduction_status: ReproductionStatus::Confirmed, + scope_class, + base_branch: "main", + } +} + +/// The orchestrator's whole reason to exist: reading LoopX's nested JSON without +/// mistaking a refusal for approval. This drives the real CLI end to end. +#[tokio::test] +async fn the_orchestrator_plans_a_bounded_issue_as_a_fix() { + let Some(loopx) = loopx_or_skip("the_orchestrator_plans_a_bounded_issue_as_a_fix") else { + return; + }; + let dir = tempfile::tempdir().expect("temp dir is created"); + let context = grounded_repository_context(); + let request = issue_request(&context, ScopeClass::Bounded); + + let outcome = IssueFixOrchestrator::new(&loopx) + .plan_issue( + &request, + env!("CARGO_MANIFEST_DIR"), + dir.path(), + // Dry run: nothing may touch the working tree in a test. + ExecutionMode::DryRun, + ) + .await + .expect("planning succeeds"); + + assert_eq!(outcome.issue_ref, "1849"); + assert_eq!(outcome.feasibility.route, FixRoute::FixPr); + assert_eq!(outcome.feasibility.next_step, NextStep::RunnableSuccessor); + assert_eq!( + outcome.feasibility.context_grounding, + ContextGrounding::Grounded + ); + assert!(!outcome.feasibility.reason_codes.is_empty()); + + let branch = outcome.branch.expect("a fix route prepares a branch"); + assert_eq!(branch.issue_branch, "codex/issue-1849-fix"); + assert_eq!(branch.base_branch, "main"); + assert_eq!(branch.branch_action, "dry_run"); + assert!(!branch.branch_ready, "a dry run creates nothing"); + assert!(!branch.validation_executed); + // The PR gate must stay shut: a dry run has neither validation nor evidence. + assert!( + !branch.may_open_pull_request(outcome.feasibility.route), + "a dry run must never permit a pull request" + ); +} + +/// An oversized scope must not even reach branch preparation. Under +/// `ExecutionMode::Execute` that would create a branch LoopX just declined to +/// justify, so the skip is a safety property, not an optimization. +#[tokio::test] +async fn the_orchestrator_skips_the_branch_on_a_triage_route() { + let Some(loopx) = loopx_or_skip("the_orchestrator_skips_the_branch_on_a_triage_route") else { + return; + }; + let dir = tempfile::tempdir().expect("temp dir is created"); + let context = grounded_repository_context(); + let request = issue_request(&context, ScopeClass::Oversized); + + let outcome = IssueFixOrchestrator::new(&loopx) + .plan_issue( + &request, + env!("CARGO_MANIFEST_DIR"), + dir.path(), + ExecutionMode::DryRun, + ) + .await + .expect("planning succeeds"); + + assert_eq!(outcome.feasibility.route, FixRoute::TriageOnly); + assert_eq!(outcome.feasibility.next_step, NextStep::NoFollowup); + assert!( + outcome.branch.is_none(), + "a declined route must not prepare a branch" + ); +} + +/// LoopX raises `user_gate` for semantic ambiguity and missing write authority. +/// The orchestrator must surface it as a distinct step a caller cannot cross. +#[tokio::test] +async fn the_orchestrator_surfaces_a_user_gate_from_a_pull_request() { + let Some(loopx) = loopx_or_skip("the_orchestrator_surfaces_a_user_gate_from_a_pull_request") + else { + return; + }; + let dir = tempfile::tempdir().expect("temp dir is created"); + + let metadata = dir.path().join("pr.json"); + std::fs::write( + &metadata, + serde_json::json!({ + "number": 9999, + "state": "OPEN", + "isDraft": false, + "mergeable": "MERGEABLE", + "reviewDecision": "", + "statusCheckRollup": [{"state": "SUCCESS"}], + }) + .to_string(), + ) + .expect("metadata is written"); + + let correction = dir.path().join("correction.json"); + std::fs::write( + &correction, + serde_json::json!({ + "schema_version": "issue_fix_maintainer_correction_input_v0", + "correction_kind": "semantic_ambiguity", + "source_kind": "maintainer_comment", + "source_ref": "GCWing/BitFun:issues/1849#comment", + "summary": "maintainer suggests highlighting the session instead of the workspace", + "user_question": "Should the arrow be removed or replaced with a check glyph?", + }) + .to_string(), + ) + .expect("correction is written"); + + // Raw call: the orchestrator's lifecycle method does not take a correction, + // so drive the CLI directly and assert the decision the orchestrator would + // then have to classify. + let packet = loopx + .issue_fix([ + "pr-lifecycle", + "--repo", + "GCWing/BitFun", + "--pr-ref", + "9999", + "--issue-ref", + "1849", + "--metadata-json", + metadata.to_str().expect("path is UTF-8"), + "--maintainer-correction-json", + correction.to_str().expect("path is UTF-8"), + "--no-write-domain-state", + ]) + .await + .expect("lifecycle projection succeeds"); + + assert_eq!(packet["transition"]["decision"], "user_gate"); + assert_eq!(packet["transition"]["role"], "user"); + assert!( + NextStep::UserGate.requires_human(), + "the orchestrator must treat this as a human gate" + ); +} + +/// The lifecycle method against a mocked PR state, through the typed path. +#[tokio::test] +async fn the_orchestrator_projects_a_merged_pull_request_as_terminal() { + let Some(loopx) = loopx_or_skip("the_orchestrator_projects_a_merged_pull_request_as_terminal") + else { + return; + }; + let dir = tempfile::tempdir().expect("temp dir is created"); + let metadata = dir.path().join("merged.json"); + std::fs::write( + &metadata, + serde_json::json!({ + "number": 9999, + "state": "MERGED", + "isDraft": false, + "reviewDecision": "APPROVED", + "statusCheckRollup": [{"state": "SUCCESS"}], + }) + .to_string(), + ) + .expect("metadata is written"); + + let outcome = IssueFixOrchestrator::new(&loopx) + .pull_request_lifecycle( + "GCWing/BitFun", + "9999", + "1849", + Some(metadata.to_str().expect("path is UTF-8")), + ) + .await + .expect("lifecycle projection succeeds"); + + assert_eq!(outcome.next_step, NextStep::NoFollowup); + assert_eq!(outcome.state, "MERGED"); + assert_eq!(outcome.state_bucket, "terminal"); + assert!(!outcome.next_step.requires_human()); +} diff --git a/src/web-ui/src/app/components/NavPanel/sections/workspaces/WorkspaceItem.tsx b/src/web-ui/src/app/components/NavPanel/sections/workspaces/WorkspaceItem.tsx index 73b6e4409e..53d56ba405 100644 --- a/src/web-ui/src/app/components/NavPanel/sections/workspaces/WorkspaceItem.tsx +++ b/src/web-ui/src/app/components/NavPanel/sections/workspaces/WorkspaceItem.tsx @@ -2,7 +2,6 @@ import React, { lazy, Suspense, useCallback, useContext, useEffect, useMemo, use import { createPortal } from 'react-dom'; import { Folder, FolderOpen, MoreHorizontal, FolderSearch, Plus, ChevronDown, Trash2, RotateCcw, Copy, FileText, Bot, Link2, ListChecks, Loader2, Clock3, ShieldCheck, Pencil, Server } from 'lucide-react'; import { useTranslation } from 'react-i18next'; -import { DotMatrixArrowRightIcon } from './DotMatrixArrowRightIcon'; import { Button, ConfirmDialog, InputDialog, Modal, Tooltip } from '@/component-library'; import { useI18n } from '@/infrastructure/i18n'; import { getAppearanceOverlayHost } from '@/infrastructure/appearance/runtime/AppearanceOverlayHost'; @@ -800,15 +799,9 @@ const WorkspaceItem: React.FC = ({ data-workspace-id={workspace.id} >