Skip to content

fix(controller): sync Human worker allowlist#972

Open
RerankerGuo wants to merge 1 commit into
agentscope-ai:mainfrom
RerankerGuo:fix/issue-954-human-worker-allowlist
Open

fix(controller): sync Human worker allowlist#972
RerankerGuo wants to merge 1 commit into
agentscope-ai:mainfrom
RerankerGuo:fix/issue-954-human-worker-allowlist

Conversation

@RerankerGuo

@RerankerGuo RerankerGuo commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • sync Human spec.accessibleWorkers into target Worker channelPolicy.groupAllowExtra so room access also grants runtime message permission
  • mark controller-managed allowlist entries with a Worker annotation so revokes/deletes only remove entries created by Human reconciliation
  • add HumanReconciler tests for add, revoke, and preserving manual allowlist entries

Fixes #954.

Tests

  • git diff --check
  • rg -n "<<<<<<<|=======|>>>>>>>" changelog/current.md hiclaw-controller/internal/controller

Local: Go 1.25.0 go test ./internal/controller passes, including the three Human allowlist regression tests.

Temporary dependency

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

❌ Integration Tests Failed (llm-interaction / mgr=copaw / wk=hermes)

Commit: fceeffd
Workflow run: #1501

Test Results
No test output captured.
Debug Log (tail)
No debug logs available.

📦 Download full debug logs & test artifacts

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

📊 CI Metrics Report

Summary

Metric Current Baseline Change
LLM Calls 139 81 +58 ↑ +71.6%
Input Tokens 4363590 2803871 +1559719 ↑ +55.6%
Output Tokens 38542 16791 +21751 ↑ +129.5%
Total Tokens 4402132 2820662 +1581470 ↑ +56.1%

By Role

Role Metric Current Baseline Change
🧠 Manager LLM Calls 83 68 +15 ↑ +22.1%
Input Tokens 2914531 2502214 +412317 ↑ +16.5%
Output Tokens 17062 13725 +3337 ↑ +24.3%
Total Tokens 2931593 2515939 +415654 ↑ +16.5%
🔧 Workers LLM Calls 56 13 +43 ↑ +330.8%
Input Tokens 1449059 301657 +1147402 ↑ +380.4%
Output Tokens 21480 3066 +18414 ↑ +600.6%
Total Tokens 1470539 304723 +1165816 ↑ +382.6%

Per-Test Breakdown

Test Mgr Calls Wkr Calls Δ Calls Mgr In Wkr In Mgr Out Wkr Out Δ Tokens Trend
02-create-worker 7 0 -5 ↓ -41.7% 182759 0 668 0 -175195 ↓ -48.9% ✅ improved
03-assign-task 10 7 +2 ↑ +13.3% 253180 164407 1714 1166 -53189 ↓ -11.2% ⚠️ regressed
04-human-intervene 16 13 +16 ↑ +123.1% 348791 333488 3009 1564 +253854 ↑ +58.6% ⚠️ regressed
05-heartbeat 5 4 +2 ↑ +28.6% 130231 89407 1230 508 -53876 ↓ -19.6% ⚠️ regressed
06-multi-worker 45 32 +43 ↑ +126.5% 1999570 861757 10441 18242 +1609876 ↑ +125.8% ⚠️ regressed

Trends

1 test(s) improved (fewer LLM calls)
⚠️ 4 test(s) regressed (more LLM calls)


Generated by HiClaw CI on 2026-07-13 16:13:15 UTC


📦 Download debug logs & test artifacts

@RerankerGuo

Copy link
Copy Markdown
Contributor Author

@shiyiyue1102 Hi! SHARD_A copaw/hermes failed on this PR's CI. Same pattern as #984 — I downloaded the artifacts from #984's run 28634278368 and confirmed: in the copaw/hermes case the hiclaw-worker-bob session directory does not exist at all and Manager never invoked hiclaw create worker --name bob — Manager only created alice. So the failure mode is the Manager not creating bob (a test-prompt interpretation flake), not anything in this PR's Human Reconciler / groupAllowExtra sync logic.

This PR's change is controller-only (Human Reconciler + Worker annotation markers); it doesn't touch the Manager agent runtime or the worker-create code path. Could you either re-run CI or merge if you agree the flake is unrelated? Happy to dig further if you can point me at what assertion actually failed.

@shiyiyue1102

Copy link
Copy Markdown
Collaborator

Synchronizing Human room access with runtime allow-lists is high value, but this branch is not independently reviewable yet. It conflicts with main and includes the unrelated multi-worker setup change. Please rebase, remove that stacked test commit, use the current AgentTeams annotation and API naming conventions, and add focused coverage for multiple Humans sharing a Worker, access revocation, Human deletion, manual allow-list entries, and idempotent reconciliation. The controller must remove only entries it owns.

Change-Id: I9796cadada04d018759e242b59390d9b7cad21c2
@shiyiyue1102
shiyiyue1102 force-pushed the fix/issue-954-human-worker-allowlist branch from 7e36aa7 to 98ea88d Compare July 25, 2026 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

您目前没有访问此智能体的权限,需要审批。 || You currently do not have permission to access this agent and need approval.

2 participants