mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-10-20 05:11:33 +00:00
test(managed-agent): close the G0 admission follow-ups (#13095)
* test(managed-agent): close the G0 admission follow-ups Post-merge follow-ups from the #12955 review, all test or documentation only: - Pin TMPDIR, TMP and TEMP to the test directory after the Hosted Harness environment is cleared, as the sibling MySQL fixture does (#13045). - Add a fast admission case with the file flag and mounts enabled: a frozen configuration with a drifted policy and a mount for another tenant on the same storage ID are both refused with 409 workspace_unavailable and persist nothing, while the same store admits the Workspace that passes every guard (#13046). - Build the Turn and Harness-startup diagnostics only when the Turn has failed or the Harness has exited, instead of on every poll (#13049). - Assert error.code alongside the 409 for the unsupported-agent, draining, unmounted and unsupported-configuration refusals (#13053). - Separate G0's production scope from the generated WebShell types, the Broker fault-gate change and the core resume-test adjustment in both language versions of the design (#13050). Not built or run locally; CI is expected to confirm. * test(managed-agent): run the gated admission case inside a transaction The test constructs its own ManagedAgentStore to enable Workspace files, so calls miss the Spring bean's @Transactional proxy, and ManagedWorkspaceRegistry.resolveForCreation refuses outside a creation transaction. CI failed on the ws-policy case with that IllegalStateException instead of the expected refusal. Wrap each call in a TransactionTemplate, as ManagedSessionLifecycleTest does. * test(managed-agent): name the refused Workspace and pin the store guard Review follow-up on this PR: - Capture the refusal with catchThrowable before asserting, so the ws-policy / ws-tenant label is kept when nothing is thrown. - Assert the store's message, which the registry's own refusal does not share, so a refusal moved to the registry layer no longer passes. - Fold registerFrozen into a five-argument register so the registry INSERT exists once in this file. --------- Co-authored-by: yiliang114 <jinjing.zzj@gmail.com>
This commit is contained in:
parent
78143fe335
commit
51b80dadbc
4 changed files with 139 additions and 15 deletions
|
|
@ -55,9 +55,28 @@ binding, not complete Workspace execution support. No UI changes are required.
|
|||
| Existing Broker/worker | Reuse production routing and fencing | Selected Runtime and persisted Workspace |
|
||||
| Contract and README | Document the narrow creation capability and remaining gates | Public REST and WebShell adapter |
|
||||
|
||||
Affected code is under `packages/sdk-java/managed-agent-server` and the private
|
||||
Hosted DTOs in `packages/sdk-java/qwencode`. No core authority, tool execution
|
||||
loop, database schema or public request field needs a new abstraction.
|
||||
Production behavior changes only under `packages/sdk-java/managed-agent-server`
|
||||
and in the private Hosted DTOs in `packages/sdk-java/qwencode`; it stays limited
|
||||
to the initial Workspace Read/Write/Edit Turn. No core authority, tool
|
||||
execution loop, database schema or public request field needs a new
|
||||
abstraction.
|
||||
|
||||
The merged change also touched three places outside that scope, none of which
|
||||
adds runtime behavior:
|
||||
|
||||
- **Generated WebShell types.** `packages/web-shell` regenerates
|
||||
`managed-agent-api.ts` from the updated OpenAPI descriptions; only the
|
||||
documentation comments change.
|
||||
- **Runtime Broker fault gate.** `DurableLocalRuntimeFaultGateTest` holds the
|
||||
worker's `execute` response in its fault proxy, so the first Broker cannot
|
||||
record the result before it is killed. The replacement Broker's `acquire`
|
||||
then settles the call through the takeover reconciliation of #12964, and the
|
||||
test asserts that outcome (`ALREADY_SETTLED`, one physical execution)
|
||||
instead of accepting either a settled or a resolved state, which depended
|
||||
on timing.
|
||||
- **Core resume test.** One `background-agent-resume.test.ts` case reports
|
||||
the Skill tool as registered so that its listing assertion does not pass
|
||||
vacuously. The override is still present on `main`.
|
||||
|
||||
## Validation and acceptance
|
||||
|
||||
|
|
|
|||
|
|
@ -30,7 +30,13 @@ G0 开放随创建会话准入的一次初始文件工具轮次,使用现有
|
|||
| 现有 Broker/worker | 复用生产路由与 fencing | 所选 Runtime 及持久 Workspace |
|
||||
| 契约与 README | 记录有限的创建能力及剩余门禁 | 公开 REST 与 WebShell 适配器 |
|
||||
|
||||
涉及代码位于 `packages/sdk-java/managed-agent-server`,以及 `packages/sdk-java/qwencode` 的私有 Hosted DTO。不需要为 core authority、工具执行循环、数据库 schema 或公开请求字段新增抽象。
|
||||
生产行为只在 `packages/sdk-java/managed-agent-server` 和 `packages/sdk-java/qwencode` 的私有 Hosted DTO 中变化,且仅限于初始 Workspace Read/Write/Edit Turn。不需要为 core authority、工具执行循环、数据库 schema 或公开请求字段新增抽象。
|
||||
|
||||
合入的改动还涉及该范围之外的三处,均不增加运行时行为:
|
||||
|
||||
- **生成的 WebShell 类型。** `packages/web-shell` 根据更新后的 OpenAPI 描述重新生成 `managed-agent-api.ts`,只有文档注释变化。
|
||||
- **Runtime Broker 故障门禁。** `DurableLocalRuntimeFaultGateTest` 在故障代理中扣住 worker 的 `execute` 响应,使第一个 Broker 在被终止前无法记录结果。随后替换 Broker 的 `acquire` 通过 #12964 的接管对账结算该调用,测试断言这一结果(`ALREADY_SETTLED`,且只有一次物理执行),而不再同时接受取决于时序的已结算或已解决两种状态。
|
||||
- **Core resume 测试。** `background-agent-resume.test.ts` 的一个用例把 Skill 工具报告为已注册,使其列表断言不会空洞通过。该覆盖目前仍在 `main` 上。
|
||||
|
||||
## 验证与验收
|
||||
|
||||
|
|
|
|||
|
|
@ -113,10 +113,16 @@ class HostedPublicWorkspaceIT {
|
|||
await().atMost(Duration.ofSeconds(35)).failFast(() -> {
|
||||
String status = jdbc.queryForObject("SELECT status FROM managed_agent_turn WHERE session_id = ?",
|
||||
String.class, session);
|
||||
assertThat(status).as("Turn: %s; events: %s; model requests: %s; Harness: %s",
|
||||
jdbc.queryForList("SELECT status, error_code FROM managed_agent_turn WHERE session_id = ?", session),
|
||||
jdbc.queryForList("SELECT event_type, data_json FROM managed_agent_event WHERE session_id = ?", session),
|
||||
modelRequests.size(), Files.readString(temporary.resolve("harness.log"))).isNotEqualTo("FAILED");
|
||||
// Diagnostics are built only when the Turn has failed, not on every poll.
|
||||
if ("FAILED".equals(status)) {
|
||||
throw new AssertionError(String.format("Turn failed. Turn: %s; events: %s; model requests: %s;"
|
||||
+ " Harness: %s",
|
||||
jdbc.queryForList("SELECT status, error_code FROM managed_agent_turn WHERE session_id = ?",
|
||||
session),
|
||||
jdbc.queryForList("SELECT event_type, data_json FROM managed_agent_event"
|
||||
+ " WHERE session_id = ?", session),
|
||||
modelRequests.size(), Files.readString(temporary.resolve("harness.log"))));
|
||||
}
|
||||
}).untilAsserted(() -> {
|
||||
if (approvals) answerActions(session, webShell);
|
||||
assertThat(modelFailure.get()).isNull();
|
||||
|
|
@ -172,16 +178,16 @@ class HostedPublicWorkspaceIT {
|
|||
.path("error").path("code").asText()).isEqualTo("unsupported_feature");
|
||||
Map<String, Object> unsupportedAgent = new java.util.LinkedHashMap<>(denied);
|
||||
unsupportedAgent.put("agent_id", "another-agent");
|
||||
request("POST", "/v1/agents/sessions", unsupportedAgent, "unsupported-agent", "actor", 409);
|
||||
assertUnavailable(request("POST", "/v1/agents/sessions", unsupportedAgent, "unsupported-agent", "actor", 409));
|
||||
Map<String, Object> unknown = new java.util.LinkedHashMap<>(denied);
|
||||
unknown.put("workspace", Map.of("workspace_id", "unknown"));
|
||||
request("POST", "/v1/agents/sessions", unknown, "unknown", "actor", 404);
|
||||
jdbc.update("UPDATE managed_workspace_registry SET state = 'DRAINING' WHERE tenant_id = ?", tenant);
|
||||
request("POST", "/v1/agents/sessions", denied, "draining", "actor", 409);
|
||||
assertUnavailable(request("POST", "/v1/agents/sessions", denied, "draining", "actor", 409));
|
||||
jdbc.update("UPDATE managed_workspace_registry SET state = 'ACTIVE' WHERE tenant_id = ?", tenant);
|
||||
register("unmounted", "unmounted-storage");
|
||||
unknown.put("workspace", Map.of("workspace_id", "unmounted"));
|
||||
request("POST", "/v1/agents/sessions", unknown, "unmounted", "actor", 409);
|
||||
assertUnavailable(request("POST", "/v1/agents/sessions", unknown, "unmounted", "actor", 409));
|
||||
var crossTenant = http.send(HttpRequest.newBuilder(URI.create("http://127.0.0.1:" + port + "/v1/agents/sessions"))
|
||||
.timeout(Duration.ofSeconds(10)).header("X-Qwen-Tenant-Id", "other-tenant")
|
||||
.header("X-G0-Fixture-Actor", "actor").build(), HttpResponse.BodyHandlers.ofString());
|
||||
|
|
@ -190,7 +196,7 @@ class HostedPublicWorkspaceIT {
|
|||
request("POST", "/v1/agents/sessions", denied, "read-only", "actor", 403);
|
||||
jdbc.update("UPDATE managed_workspace_access SET can_create = TRUE WHERE tenant_id = ?", tenant);
|
||||
jdbc.update("UPDATE managed_workspace_registry SET config_ref = 'unsupported' WHERE tenant_id = ?", tenant);
|
||||
request("POST", "/v1/agents/sessions", denied, "unsupported", "actor", 409);
|
||||
assertUnavailable(request("POST", "/v1/agents/sessions", denied, "unsupported", "actor", 409));
|
||||
assertThat(jdbc.queryForObject("SELECT COUNT(*) FROM managed_agent_session WHERE tenant_id = ?",
|
||||
Integer.class, tenant)).isEqualTo(2);
|
||||
assertThat(modelRequests).hasSize(8);
|
||||
|
|
@ -320,11 +326,16 @@ class HostedPublicWorkspaceIT {
|
|||
environment.putAll(Map.of("HOME", home.toString(), "USERPROFILE", home.toString(), "QWEN_HOME", config.toString(),
|
||||
"QWEN_RUNTIME_DIR", temporary.resolve("runtime").toString(), "OPENAI_API_KEY", "fake-local-key",
|
||||
"OPENAI_BASE_URL", modelUrl, "QWEN_SERVER_TOKEN", TOKEN, "QWEN_HOSTED_HARNESS_CAPABILITY_DIGEST", DIGEST));
|
||||
environment.put("TMPDIR", temporary.toString());
|
||||
environment.put("TMP", temporary.toString());
|
||||
environment.put("TEMP", temporary.toString());
|
||||
for (String name : List.of("QWEN_CODE_SYSTEM_SETTINGS_PATH", "QWEN_CODE_SYSTEM_DEFAULTS_PATH",
|
||||
"QWEN_CODE_TRUSTED_FOLDERS_PATH")) environment.put(name, temporary.resolve(name + ".json").toString());
|
||||
harness = builder.start();
|
||||
await().atMost(Duration.ofSeconds(30)).ignoreExceptions().untilAsserted(() -> {
|
||||
assertThat(harness.isAlive()).as(Files.readString(log)).isTrue();
|
||||
if (!harness.isAlive()) {
|
||||
throw new AssertionError("Hosted Harness exited: " + Files.readString(log));
|
||||
}
|
||||
assertThat(http.send(HttpRequest.newBuilder(URI.create("http://127.0.0.1:" + harnessPort + "/capabilities"))
|
||||
.timeout(Duration.ofSeconds(2)).header("Authorization", "Bearer " + TOKEN).build(),
|
||||
HttpResponse.BodyHandlers.discarding()).statusCode()).isEqualTo(200);
|
||||
|
|
@ -389,6 +400,11 @@ class HostedPublicWorkspaceIT {
|
|||
return json.readTree(response.body());
|
||||
}
|
||||
|
||||
// Every creation refusal here shares one declared code; the fixture, not the code, selects the branch.
|
||||
private static void assertUnavailable(JsonNode refusal) {
|
||||
assertThat(refusal.path("error").path("code").asText()).isEqualTo("workspace_unavailable");
|
||||
}
|
||||
|
||||
private static int freePort() throws IOException {
|
||||
try (ServerSocket socket = new ServerSocket(0)) { return socket.getLocalPort(); }
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ package com.alibaba.qwen.code.managedagent;
|
|||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.Assertions.assertThatThrownBy;
|
||||
import static org.assertj.core.api.Assertions.catchThrowable;
|
||||
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
|
||||
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.patch;
|
||||
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
|
||||
|
|
@ -17,6 +18,7 @@ import com.alibaba.qwen.code.managedagent.service.ManagedAgentService;
|
|||
import com.alibaba.qwen.code.managedagent.store.ManagedAgentStore;
|
||||
import com.alibaba.qwen.code.managedagent.store.ManagedWorkspaceRegistry;
|
||||
import com.alibaba.qwen.code.managedagent.store.StoreModels.SessionMutationKind;
|
||||
import com.alibaba.qwen.code.runtimebroker.WorkspaceExecutionProfile;
|
||||
import com.alibaba.qwen.code.runtimebroker.managedworkspace.ContextBinding;
|
||||
import com.fasterxml.jackson.databind.ObjectMapper;
|
||||
import java.time.Clock;
|
||||
|
|
@ -33,9 +35,12 @@ import org.springframework.beans.factory.annotation.Autowired;
|
|||
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
|
||||
import org.springframework.boot.test.context.SpringBootTest;
|
||||
import org.springframework.dao.DataIntegrityViolationException;
|
||||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.http.MediaType;
|
||||
import org.springframework.jdbc.core.JdbcTemplate;
|
||||
import org.springframework.test.web.servlet.MockMvc;
|
||||
import org.springframework.transaction.PlatformTransactionManager;
|
||||
import org.springframework.transaction.support.TransactionTemplate;
|
||||
|
||||
@SpringBootTest(properties = {
|
||||
"spring.datasource.url=jdbc:h2:mem:workspace-admission;MODE=MySQL;"
|
||||
|
|
@ -65,6 +70,9 @@ class ManagedWorkspaceAdmissionTest {
|
|||
@Autowired
|
||||
private ManagedWorkspaceRegistry registry;
|
||||
|
||||
@Autowired
|
||||
private PlatformTransactionManager transactionManager;
|
||||
|
||||
@Test
|
||||
void discoveryFiltersBeforePagingAndKeepsDefaultOutsidePage()
|
||||
throws Exception {
|
||||
|
|
@ -523,6 +531,77 @@ class ManagedWorkspaceAdmissionTest {
|
|||
Integer.class, tenant)).isZero();
|
||||
}
|
||||
|
||||
@Test
|
||||
void enabledCreationRefusesPolicyDriftAndAnotherTenantsMount() {
|
||||
String tenant = "tenant-" + UUID.randomUUID();
|
||||
String otherTenant = "tenant-" + UUID.randomUUID();
|
||||
// Each Workspace differs from the admitted one in exactly one guard.
|
||||
register(tenant, "ws-valid", "storage-valid",
|
||||
WorkspaceExecutionProfile.CONFIG_REF,
|
||||
WorkspaceExecutionProfile.POLICY_REF);
|
||||
register(tenant, "ws-policy", "storage-policy",
|
||||
WorkspaceExecutionProfile.CONFIG_REF,
|
||||
"preapproved-workspace-tools/2");
|
||||
register(tenant, "ws-tenant", "storage-shared",
|
||||
WorkspaceExecutionProfile.CONFIG_REF,
|
||||
WorkspaceExecutionProfile.POLICY_REF);
|
||||
for (String workspace : List.of("ws-valid", "ws-policy", "ws-tenant")) {
|
||||
grant(tenant, workspace, "actor-a", true);
|
||||
}
|
||||
ManagedAgentProperties enabled = new ManagedAgentProperties();
|
||||
enabled.getHarness().setWorkspaceFilesEnabled(true);
|
||||
enabled.getRuntimeBroker().setWorkspaceMounts(List.of(
|
||||
new ManagedAgentProperties.RuntimeBroker.WorkspaceMount(
|
||||
tenant, "storage-valid", "/unused/valid"),
|
||||
new ManagedAgentProperties.RuntimeBroker.WorkspaceMount(
|
||||
tenant, "storage-policy", "/unused/policy"),
|
||||
new ManagedAgentProperties.RuntimeBroker.WorkspaceMount(
|
||||
otherTenant, "storage-shared", "/unused/shared")));
|
||||
ManagedAgentStore gated = new ManagedAgentStore(jdbc, mapper,
|
||||
Clock.systemUTC(), ignored -> {
|
||||
}, registry, enabled);
|
||||
// An unproxied store has no @Transactional; creation resolves the
|
||||
// Workspace only inside a transaction, as the Spring bean provides.
|
||||
TransactionTemplate transaction = new TransactionTemplate(
|
||||
transactionManager);
|
||||
List<Map<String, Object>> input = List.of(
|
||||
Map.of("type", "text", "text", "go"));
|
||||
String digest = "sha256:" + "a".repeat(64);
|
||||
|
||||
for (String workspace : List.of("ws-policy", "ws-tenant")) {
|
||||
// Captured first so the label survives when nothing is thrown.
|
||||
Throwable thrown = catchThrowable(() -> transaction.execute(status ->
|
||||
gated.insertWorkspaceSessionCommand(tenant, "actor-a",
|
||||
workspace, digest, "qwen-code", null, null, input,
|
||||
digest, new WorkspaceSelection(workspace, "."))));
|
||||
assertThat(thrown).as(workspace)
|
||||
.isInstanceOfSatisfying(ApiException.class, error -> {
|
||||
// The store's message, not the registry's "Workspace is
|
||||
// unavailable.", ties the refusal to the store guard.
|
||||
assertThat(error.getMessage()).isEqualTo(
|
||||
"Hosted Workspace execution is not available.");
|
||||
assertThat(error.getStatus())
|
||||
.isEqualTo(HttpStatus.CONFLICT);
|
||||
assertThat(error.getCode())
|
||||
.isEqualTo("workspace_unavailable");
|
||||
});
|
||||
}
|
||||
assertThat(jdbc.queryForObject("SELECT COUNT(*) FROM"
|
||||
+ " managed_agent_session WHERE tenant_id = ?",
|
||||
Integer.class, tenant)).isZero();
|
||||
assertThat(jdbc.queryForObject("SELECT COUNT(*) FROM"
|
||||
+ " managed_agent_turn WHERE tenant_id = ?",
|
||||
Integer.class, tenant)).isZero();
|
||||
|
||||
// The same store admits the Workspace that passes every guard, so the
|
||||
// refusals above come from the policy and mount-tenant checks alone.
|
||||
assertThat(transaction.execute(status ->
|
||||
gated.insertWorkspaceSessionCommand(tenant, "actor-a",
|
||||
"ws-valid", digest, "qwen-code", null, null, input,
|
||||
digest, new WorkspaceSelection("ws-valid", ".")))
|
||||
.sessionId()).isNotBlank();
|
||||
}
|
||||
|
||||
@Test
|
||||
void webShellCreationIsMetadataOnlyUntilExecutionIsWired()
|
||||
throws Exception {
|
||||
|
|
@ -761,12 +840,16 @@ class ManagedWorkspaceAdmissionTest {
|
|||
}
|
||||
|
||||
private void register(String tenant, String id, String storageId) {
|
||||
register(tenant, id, storageId, "config-" + id, "policy-" + id);
|
||||
}
|
||||
|
||||
private void register(String tenant, String id, String storageId,
|
||||
String configRef, String policyRef) {
|
||||
jdbc.update("INSERT INTO managed_workspace_registry (tenant_id,"
|
||||
+ " workspace_id, workspace_generation, storage_id,"
|
||||
+ " display_name, config_ref, policy_ref, state)"
|
||||
+ " VALUES (?, ?, 1, ?, ?, ?, ?, 'ACTIVE')",
|
||||
tenant, id, storageId, id, "config-" + id,
|
||||
"policy-" + id);
|
||||
tenant, id, storageId, id, configRef, policyRef);
|
||||
}
|
||||
|
||||
private void grant(String tenant, String workspaceId, String actorId,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue