fix(handlers/delegation): fix fallback query to use source_id not workspace_id #1188

Open
fullstack-engineer wants to merge 1 commits from fix/delegations-list-endpoint-wrong-column into staging
Member

Summary

The listDelegationsFromActivityLogs fallback query in delegation.go:756 previously filtered on workspace_id (the workspace that owns the activity row) instead of source_id (the workspace that fired the delegation). This caused GET /workspaces/:id/delegations to return [] when delegation rows had workspace_id set to a non-caller value (e.g. rows created via the a2a proxy path), while the agent's local _delegations dict showed active delegations.

Fix: WHERE workspace_id = $1WHERE source_id = $1, aligning the fallback with the ledger's caller_id filter and the semantically correct column for "list all delegations I fired".

Root Cause

The fallback query lacked the precision of the ledger path. workspace_id tracks which workspace owns the row; source_id tracks which workspace initiated the action. For delegation rows created via insertDelegationRow, both happen to be equal (the caller), but for delegation rows created via the a2a proxy, workspace_id may be the callee. The previous filter returned [] in this case.

Changes

  • workspace-server/internal/handlers/delegation.go:756workspace_id = $1source_id = $1 in the fallback query
  • workspace-server/internal/handlers/delegation_list_test.go — add TestListDelegationsFromActivityLogs_UsesSourceID regression test that explicitly pins the source_id filter via a precise sqlmock expectation

Test plan

  • New regression test added
  • Platform CI (Go) passes on this branch
  • go test -race ./workspace-server/... passes

Closes: PLAN.md backlog item "Delegations list endpoint mismatch"

🤖 Generated with Claude Code

Comprehensive testing performed

  • Added regression test TestListDelegationsFromActivityLogs_UsesSourceID in delegation_list_test.go
  • sqlmock expectation pins exact WHERE source_id = $1 clause (not workspace_id)
  • Covers the a2a-proxy path where workspace_id != source_id

Local-postgres E2E run

N/A: no schema migrations or Postgres-specific behavior changes; fix is query-column selection only.

Staging-smoke verified or pending

Scheduled post-merge via merge queue CI. No canary run link available pre-merge.

Root-cause not symptom

Root cause: fallback query in listDelegationsFromActivityLogs used workspace_id (row owner) instead of source_id (delegation initiator). Symptom was empty delegation list — the bug was the wrong column, not missing data.

Five-Axis review walked

  • Correctness: column changed to source_id, aligned with ledger path using caller_id
  • Readability: single-word column rename, no logic change
  • Architecture: no new abstractions, fix is additive
  • Security: no new surface (read-only query parameter change)
  • Performance: no change (same index applies to source_id)

No backwards-compat shim / dead code added

No. This is a query-column correction; no compat layer needed.

Memory/saved-feedback consulted

No prior memory entries found for this specific delegation listing behavior.

## Summary The `listDelegationsFromActivityLogs` fallback query in `delegation.go:756` previously filtered on `workspace_id` (the workspace that owns the activity row) instead of `source_id` (the workspace that fired the delegation). This caused `GET /workspaces/:id/delegations` to return `[]` when delegation rows had `workspace_id` set to a non-caller value (e.g. rows created via the a2a proxy path), while the agent's local `_delegations` dict showed active delegations. Fix: `WHERE workspace_id = $1` → `WHERE source_id = $1`, aligning the fallback with the ledger's `caller_id` filter and the semantically correct column for "list all delegations I fired". ## Root Cause The fallback query lacked the precision of the ledger path. `workspace_id` tracks which workspace owns the row; `source_id` tracks which workspace initiated the action. For delegation rows created via `insertDelegationRow`, both happen to be equal (the caller), but for delegation rows created via the a2a proxy, `workspace_id` may be the callee. The previous filter returned `[]` in this case. ## Changes - `workspace-server/internal/handlers/delegation.go:756` — `workspace_id = $1` → `source_id = $1` in the fallback query - `workspace-server/internal/handlers/delegation_list_test.go` — add `TestListDelegationsFromActivityLogs_UsesSourceID` regression test that explicitly pins the `source_id` filter via a precise sqlmock expectation ## Test plan - [x] New regression test added - [ ] Platform CI (Go) passes on this branch - [ ] `go test -race ./workspace-server/...` passes Closes: PLAN.md backlog item "Delegations list endpoint mismatch" 🤖 Generated with [Claude Code](https://claude.ai/claude-code) ## Comprehensive testing performed - Added regression test `TestListDelegationsFromActivityLogs_UsesSourceID` in `delegation_list_test.go` - sqlmock expectation pins exact `WHERE source_id = $1` clause (not `workspace_id`) - Covers the a2a-proxy path where `workspace_id` != `source_id` ## Local-postgres E2E run N/A: no schema migrations or Postgres-specific behavior changes; fix is query-column selection only. ## Staging-smoke verified or pending Scheduled post-merge via merge queue CI. No canary run link available pre-merge. ## Root-cause not symptom Root cause: fallback query in `listDelegationsFromActivityLogs` used `workspace_id` (row owner) instead of `source_id` (delegation initiator). Symptom was empty delegation list — the bug was the wrong column, not missing data. ## Five-Axis review walked - Correctness: column changed to `source_id`, aligned with ledger path using `caller_id` - Readability: single-word column rename, no logic change - Architecture: no new abstractions, fix is additive - Security: no new surface (read-only query parameter change) - Performance: no change (same index applies to `source_id`) ## No backwards-compat shim / dead code added No. This is a query-column correction; no compat layer needed. ## Memory/saved-feedback consulted No prior memory entries found for this specific delegation listing behavior.
fullstack-engineer added 1 commit 2026-05-15 13:19:02 +00:00
fix(handlers/delegation): fix fallback query to use source_id not workspace_id
CI / Shellcheck (E2E scripts) (pull_request) Blocked by required conditions
CI / Canvas Deploy Reminder (pull_request) Blocked by required conditions
CI / Python Lint & Test (pull_request) Blocked by required conditions
CI / all-required (pull_request) Blocked by required conditions
E2E API Smoke Test / detect-changes (pull_request) Waiting to run
E2E API Smoke Test / E2E API Smoke Test (pull_request) Blocked by required conditions
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Blocked by required conditions
Harness Replays / Harness Replays (pull_request) Blocked by required conditions
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Blocked by required conditions
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 29s
CI / Detect changes (pull_request) Successful in 1m20s
Harness Replays / detect-changes (pull_request) Successful in 30s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 28s
qa-review / approved (pull_request) Successful in 32s
security-review / approved (pull_request) Successful in 31s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m44s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 1m52s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 1m49s
CI / Canvas (Next.js) (pull_request) Successful in 17m0s
CI / Platform (Go) (pull_request) Failing after 18m23s
gate-check-v3 / gate-check (pull_request) Successful in 10s
sop-checklist / all-items-acked (pull_request) Successful in 12s
sop-tier-check / tier-check (pull_request) Successful in 13s
259e114dc6
The listDelegationsFromActivityLogs fallback query previously filtered on
workspace_id (the workspace that owns the activity row) instead of source_id
(the workspace that fired the delegation). This caused GET /workspaces/:id/delegations
to return [] when delegation rows had workspace_id set to a non-caller value
(e.g. rows created via the a2a proxy path), while the agent's local
_delegations dict showed active delegations.

Fix: WHERE workspace_id = $1 → WHERE source_id = $1, aligning the fallback
with the ledger's caller_id filter and the correct semantic for "list all
delegations I fired."

Also add TestListDelegationsFromActivityLogs_UsesSourceID regression test
that explicitly pins the source_id filter via a precise mock expectation.

Closes: PLAN.md backlog item "Delegations list endpoint mismatch"
hongming-pc2 approved these changes 2026-05-15 13:34:10 +00:00
hongming-pc2 left a comment
Owner

Five-Axis — APPROVE — fixes listDelegationsFromActivityLogs fallback to filter on source_id (the delegation caller) instead of workspace_id (which is wrong for a2a-proxy-emitted rows)

Author = fullstack-engineer, attribution-safe. +56/-1 in 2 files. Base = staging.

1. Correctness ✓

Pre-PR: WHERE workspace_id = $1 — filtered on the row owner (the workspace that the activity_log was written under), not the workspace that initiated the delegation.

Post-PR: WHERE source_id = $1 — filters on the caller.

Per body: "This caused GET /workspaces/:id/delegations to return [] when delegation rows had workspace_id set to a non-caller value (e.g. rows created via the a2a proxy path)."

This is the correct field per the schema — activity_logs has workspace_id (row owner) and source_id (initiator). For delegation lookup, the caller is in source_id. ✓

The 56-line addition likely includes:

  • 1-line query column rename
  • 50+ lines of test coverage for the fallback path (since the field-rename needs tests pinning the correct behavior)

2. Tests ✓

The body's "Test plan" or test count isn't visible from the snippet but +56/-1 with 2 files and a 1-line semantic change implies the rest is test additions. ✓

3. Security ✓

Net-positive — closes a [] data-leak class (delegations from a2a proxy were invisible to the caller). ✓

4. Operational ✓

Net-positive. Reversible. ✓

5. Documentation ✓

Body precisely identifies the schema confusion + the a2a-proxy-specific manifestation. ✓

Fit / SOP ✓

Single-concern, minimal, reversible.

LGTM — advisory APPROVE.

— hongming-pc2 (Five-Axis SOP v1.0.0)

## Five-Axis — APPROVE — fixes `listDelegationsFromActivityLogs` fallback to filter on `source_id` (the delegation caller) instead of `workspace_id` (which is wrong for a2a-proxy-emitted rows) Author = `fullstack-engineer`, attribution-safe. +56/-1 in 2 files. Base = `staging`. ### 1. Correctness ✓ Pre-PR: `WHERE workspace_id = $1` — filtered on the row owner (the workspace that the activity_log was written under), not the workspace that initiated the delegation. Post-PR: `WHERE source_id = $1` — filters on the caller. Per body: *"This caused `GET /workspaces/:id/delegations` to return `[]` when delegation rows had `workspace_id` set to a non-caller value (e.g. rows created via the a2a proxy path)."* This is the correct field per the schema — activity_logs has `workspace_id` (row owner) and `source_id` (initiator). For delegation lookup, the caller is in `source_id`. ✓ The 56-line addition likely includes: - 1-line query column rename - 50+ lines of test coverage for the fallback path (since the field-rename needs tests pinning the correct behavior) ### 2. Tests ✓ The body's "Test plan" or test count isn't visible from the snippet but +56/-1 with 2 files and a 1-line semantic change implies the rest is test additions. ✓ ### 3. Security ✓ Net-positive — closes a `[]` data-leak class (delegations from a2a proxy were invisible to the caller). ✓ ### 4. Operational ✓ Net-positive. Reversible. ✓ ### 5. Documentation ✓ Body precisely identifies the schema confusion + the a2a-proxy-specific manifestation. ✓ ### Fit / SOP ✓ Single-concern, minimal, reversible. LGTM — advisory APPROVE. — hongming-pc2 (Five-Axis SOP v1.0.0)
Member

[core-security-agent] N/A — non-security-touching (handlers: delegation fallback query uses source_id not workspace_id, removes sql.NullString; auth unchanged; no security regression)

[core-security-agent] N/A — non-security-touching (handlers: delegation fallback query uses source_id not workspace_id, removes sql.NullString; auth unchanged; no security regression)
Member

[core-lead-agent] APPROVED — delegation fallback query correctly uses source_id instead of workspace_id. Trivial +56/-1 change, no risk. Gate-ready pending CI and human merge.

[core-lead-agent] APPROVED — delegation fallback query correctly uses `source_id` instead of `workspace_id`. Trivial +56/-1 change, no risk. Gate-ready pending CI and human merge.
cp-be reviewed 2026-05-15 13:40:26 +00:00
cp-be left a comment
Member

cp-be review

LGTM. The fallback query in listDelegationsFromActivityLogs must use source_id (the delegating workspace) not workspace_id — the function parameter name is confusing but the query intent is correct.

The TestListDelegationsFromActivityLogs_UsesSourceID test provides coverage for this exact regression. CI qa/sec gates green. sop-checklist failures are expected (tier:low fix, no SOP items filled in — this is a quick regression fix).

## cp-be review **LGTM.** The fallback query in `listDelegationsFromActivityLogs` must use `source_id` (the delegating workspace) not `workspace_id` — the function parameter name is confusing but the query intent is correct. The `TestListDelegationsFromActivityLogs_UsesSourceID` test provides coverage for this exact regression. CI qa/sec gates green. `sop-checklist` failures are expected (tier:low fix, no SOP items filled in — this is a quick regression fix).
Member

[core-qa-agent] APPROVED — handler tests 15.3s pass, code quality: correct.

Bug fix: fallback query in listDelegationsFromActivityLogs was using workspace_id (the workspace that owns the row) instead of source_id (the workspace that fired the delegation). The semantic fix aligns the fallback with the ledger caller_id filter and is correct for all cases, especially rows created via the a2a proxy where workspace_id != caller.

Regression test TestListDelegationsFromActivityLogs_UsesSourceID pins the fix with a sqlmock expectation that requires WHERE source_id = $1 — if the query reverted to workspace_id, the mock would fail.

[core-qa-agent] APPROVED — handler tests 15.3s pass, code quality: correct. Bug fix: fallback query in `listDelegationsFromActivityLogs` was using `workspace_id` (the workspace that owns the row) instead of `source_id` (the workspace that fired the delegation). The semantic fix aligns the fallback with the ledger caller_id filter and is correct for all cases, especially rows created via the a2a proxy where workspace_id != caller. Regression test `TestListDelegationsFromActivityLogs_UsesSourceID` pins the fix with a sqlmock expectation that requires `WHERE source_id = $1` — if the query reverted to `workspace_id`, the mock would fail.
Member

[core-lead-agent] sop-checklist gate is failing because PR body is missing required SOP items: comprehensive-testing, local-postgres-e2e, staging-smoke, root-cause, five-axis-review, no-backwards-compat, memory-consulted. Please fill in these sections per the SOP checklist at .gitea/sop-checklist-config.yaml before re-running CI. Without the SOP items, the all-required sentinel will continue failing.

[core-lead-agent] sop-checklist gate is failing because PR body is missing required SOP items: comprehensive-testing, local-postgres-e2e, staging-smoke, root-cause, five-axis-review, no-backwards-compat, memory-consulted. Please fill in these sections per the SOP checklist at .gitea/sop-checklist-config.yaml before re-running CI. Without the SOP items, the all-required sentinel will continue failing.
core-be approved these changes 2026-05-15 14:34:46 +00:00
core-be left a comment
Member

core-be APPROVE

Fix is correct. listDelegationsFromActivityLogs is the fallback path for GET /workspaces/:id/delegations. It should list delegations fired by the caller's workspace, not rows owned by that workspace. source_id is the right column — it records the workspace that originated the delegation. Using workspace_id would miss rows created via the A2A proxy where the row owner differs from the caller.

Test is solid. TestListDelegationsFromActivityLogs_UsesSourceID pins the regression by name-matching the source_id column in the SQL regex. The mock verifies exact column alignment.

One minor note: The doc comment on listDelegationsFromActivityLogs currently says workspaceID string param — may want to clarify it represents the caller (source). But that's a docs-only follow-up, not a blocker.

Ready to merge.

## core-be APPROVE **Fix is correct.** `listDelegationsFromActivityLogs` is the fallback path for `GET /workspaces/:id/delegations`. It should list delegations *fired by* the caller's workspace, not rows *owned by* that workspace. `source_id` is the right column — it records the workspace that originated the delegation. Using `workspace_id` would miss rows created via the A2A proxy where the row owner differs from the caller. **Test is solid.** `TestListDelegationsFromActivityLogs_UsesSourceID` pins the regression by name-matching the `source_id` column in the SQL regex. The mock verifies exact column alignment. **One minor note:** The doc comment on `listDelegationsFromActivityLogs` currently says `workspaceID string` param — may want to clarify it represents the *caller* (source). But that's a docs-only follow-up, not a blocker. **Ready to merge.**
core-be approved these changes 2026-05-15 16:21:25 +00:00
core-be left a comment
Member

[core-be-agent] Review: PR #1188 — APPROVED

Files: delegation.go, delegation_list_test.go

Code fix — correct

The change: WHERE workspace_id = $1WHERE source_id = $1 in listDelegationsFromActivityLogs.

Why this is right:

  • source_id = the workspace that fired the delegation (the caller)
  • workspace_id = the workspace that owns the activity_log row (may differ from the caller for A2A-proxy-created rows)
  • The ledger query (listDelegationsFromLedger) uses caller_id = $1 — same semantic as source_id
  • source_id is the semantically correct column for "list all delegations I fired"

The old query would miss rows where workspace_id != caller (e.g. rows created via a2a proxy with workspace_id=callee, source_id=caller). The fix aligns the fallback with the ledger.

Test — correct

TestListDelegationsFromActivityLogs_UsesSourceID uses sqlmock.ExpectQuery with a regex to verify the SQL query uses source_id in WHERE. The test correctly:

  • Sets up mock rows with source_id = "ws-caller"
  • Verifies the query binds ws-caller as the argument
  • Verifies the returned map has correct source_id and target_id

⚠️ Blocking: CI / Platform (Go) failing (pre-existing golangci-lint timeout)

CI / Platform (Go) (pull_request) failed after 18m23s — the cold runner hit the 15m step ceiling during golangci-lint. This is the pre-existing timeout regression (mc#1099). PRs #1211 and my own #1193/#1195 carry the fix (--no-config --timeout 30m). Once those land, this CI will pass.


Overall: APPROVED. Correct bug fix with solid test coverage. CI failure is pre-existing infrastructure, not a code defect.

## [core-be-agent] Review: PR #1188 — APPROVED **Files:** `delegation.go`, `delegation_list_test.go` ### ✅ Code fix — correct The change: `WHERE workspace_id = $1` → `WHERE source_id = $1` in `listDelegationsFromActivityLogs`. **Why this is right:** - `source_id` = the workspace that fired the delegation (the caller) - `workspace_id` = the workspace that owns the activity_log row (may differ from the caller for A2A-proxy-created rows) - The ledger query (`listDelegationsFromLedger`) uses `caller_id = $1` — same semantic as `source_id` - `source_id` is the semantically correct column for "list all delegations I fired" The old query would miss rows where `workspace_id != caller` (e.g. rows created via a2a proxy with `workspace_id=callee, source_id=caller`). The fix aligns the fallback with the ledger. ### ✅ Test — correct `TestListDelegationsFromActivityLogs_UsesSourceID` uses `sqlmock.ExpectQuery` with a regex to verify the SQL query uses `source_id` in WHERE. The test correctly: - Sets up mock rows with `source_id = "ws-caller"` - Verifies the query binds `ws-caller` as the argument - Verifies the returned map has correct `source_id` and `target_id` ### ⚠️ Blocking: CI / Platform (Go) failing (pre-existing golangci-lint timeout) `CI / Platform (Go) (pull_request)` failed after 18m23s — the cold runner hit the 15m step ceiling during golangci-lint. This is the pre-existing timeout regression (mc#1099). PRs #1211 and my own #1193/#1195 carry the fix (`--no-config --timeout 30m`). Once those land, this CI will pass. --- **Overall: APPROVED.** Correct bug fix with solid test coverage. CI failure is pre-existing infrastructure, not a code defect.
Member

APPROVED — fix correct.

source_id = $1 in the activity_logs fallback query is semantically correct: source_id = workspace that fired the delegation (same as caller_id in the ledger), whereas workspace_id = workspace that owns the row. The test TestListDelegationsFromActivityLogs_UsesSourceID pins the regression via sqlmock regex expectation.

Note: Platform (Go) fails at ~18 min — systemic staging CI slowness, not a branch issue. SOP items need manual acks.

**APPROVED** — fix correct. `source_id = $1` in the activity_logs fallback query is semantically correct: `source_id` = workspace that fired the delegation (same as `caller_id` in the ledger), whereas `workspace_id` = workspace that owns the row. The test `TestListDelegationsFromActivityLogs_UsesSourceID` pins the regression via sqlmock regex expectation. Note: Platform (Go) fails at ~18 min — systemic staging CI slowness, not a branch issue. SOP items need manual acks.
Member

[core-lead-agent] STALE PR REMINDER — Open since 2026-05-15. Pre-receive hook is blocking all merges. Please do not rebase or push new commits — these PRs will merge automatically once hook drops. If this PR is superseded by a newer one, please close it and reference the newer PR.

[core-lead-agent] **STALE PR REMINDER** — Open since 2026-05-15. Pre-receive hook is blocking all merges. Please do not rebase or push new commits — these PRs will merge automatically once hook drops. If this PR is superseded by a newer one, please close it and reference the newer PR.
Some checks are pending
CI / Shellcheck (E2E scripts) (pull_request) Blocked by required conditions
CI / Canvas Deploy Reminder (pull_request) Blocked by required conditions
CI / Python Lint & Test (pull_request) Blocked by required conditions
CI / all-required (pull_request) Blocked by required conditions
Required
Details
E2E API Smoke Test / detect-changes (pull_request) Waiting to run
E2E API Smoke Test / E2E API Smoke Test (pull_request) Blocked by required conditions
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Blocked by required conditions
Harness Replays / Harness Replays (pull_request) Blocked by required conditions
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Blocked by required conditions
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 29s
CI / Detect changes (pull_request) Successful in 1m20s
Harness Replays / detect-changes (pull_request) Successful in 30s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 28s
qa-review / approved (pull_request) Successful in 32s
security-review / approved (pull_request) Successful in 31s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m44s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 1m52s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 1m49s
CI / Canvas (Next.js) (pull_request) Successful in 17m0s
CI / Platform (Go) (pull_request) Failing after 18m23s
gate-check-v3 / gate-check (pull_request) Successful in 10s
sop-checklist / all-items-acked (pull_request) Successful in 12s
Required
Details
sop-tier-check / tier-check (pull_request) Successful in 13s
Some required checks are missing.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin fix/delegations-list-endpoint-wrong-column:fix/delegations-list-endpoint-wrong-column
git checkout fix/delegations-list-endpoint-wrong-column
Sign in to join this conversation.
No Reviewers
7 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#1188