feat(linear): v1.1 polish — pre-container feedback, state-on-start, sweep, nits by isadeks · Pull Request #87 · aws-samples/sample-autonomous-cloud-coding-agents

isadeks · 2026-05-13T22:35:36Z

Summary

Wraps the v1.1 polish theme for the Linear integration, addressing the silent-drop gaps and PR #63 review recommendations in a single PR. Six rejection paths now produce user-visible feedback; the issue state transitions on agent-start (not just at PR-open); a stale-reaction sweep keeps re-runs clean; and four small review nits are addressed.

What changed

1. Pre-container failure feedback (5 paths in processor + 1 in orchestrator)

Linear-triggered tasks that fail before the agent container starts now produce a Linear comment + ❌ reaction within ~20s of label-apply. Previously, six distinct rejection points all silently dropped:

#	Trigger	Layer	Message
1	Issue has no `projectId`	processor	"isn't in a project — ABCA needs a Linear project to route tasks to a repo"
2	Project not onboarded / removed	processor	"isn't onboarded; admin can run `bgagent linear onboard-project`"
3	Webhook missing organization or actor	processor	diagnostic ("report to your ABCA admin")
4	Linear actor has no linked platform user	processor	"v1 only the API-token owner can submit; multi-user OAuth on v3 roadmap"
5	`createTaskCore` returns non-201	processor	branched on status: guardrail/validation surfaces error string; 503 prompts retry; other 4xx/5xx generic
6	User concurrency cap rejection	orchestrator	"concurrency limit; wait for one to finish, then re-apply the label"

All six call sites use a shared helper cdk/src/handlers/shared/linear-feedback.ts — reportIssueFailure(secretArn, issueId, message) posts the comment and reaction in parallel via Promise.allSettled. Reuses the existing 5-minute getLinearSecret cache.

2. State transition at agent-start

Mirrors the existing In Review transition that fires at PR-open. The prompt addendum's step 1 now instructs the agent to transition Backlog/Todo → In Progress before doing repo work, so Linear's state column reflects "being worked" during the minutes between label-apply and PR-open. Falls back to Todo if In Progress doesn't exist; skips if the issue is already in In Progress or any later state. Doesn't loop on list_issue_statuses.

3. Stale-reaction sweep

Re-runs (label removed and re-applied; or pre-container ❌ followed by a successful retry) used to leave prior 👀/✅/❌ accumulated next to the new run's reaction. Added _sweep_stale_reactions(issue_id) to agent/src/linear_reactions.py:

Architecture: 👀-first, sweep-async — the user-visible 👀 fires first (~150ms typical). The sweep runs on a daemon thread after the 👀 lands, so cold-network latency (Linear's first call from a fresh container can be ~5s) doesn't gate the user-visible signal or block the agent pipeline.
Filters by viewer + emoji — only deletes the bot's own 👀/✅/❌, never touches reactions a human added even if they happen to use the same emoji.
Excludes the just-posted 👀 from its own sweep (passes exclude_id=rid).
Best-effort — failure leaves the visual-duplication bug we set out to fix, but the pipeline proceeds unaffected.

Smoke-tested on backgroundagent-dev: react_task_started total = 156ms; sweep done in background at +303ms; agent started at +3ms after react_task_started exit.

4. PR #63 review nits (4)

Auth circuit breaker in linear_reactions.py — track consecutive 401/403s; after 3 strikes, ERROR once and short-circuit until container restart. Replaces the prior behaviour where revoked tokens flooded CloudWatch with WARNs forever.
Explicit linear_eyes_reaction_id: str | None = None init in pipeline.py instead of locals().get(...) in the crash handler.
Narrow except Exception in resolve_linear_api_token to (BotoCoreError, ClientError). Switch print() → shell.log("WARN", ...).
Admin-assisted fallback docs for bgagent linear setup auto-link failures. Replaces misleading "run bgagent linear link <code>" suggestion (the command is non-functional in v1) with explicit DynamoDB put-item steps and a clear v3-OAuth note.

Plumbing

linear-integration.ts — wires LINEAR_API_TOKEN_SECRET_ARN into the webhook processor + grants read on LinearIntegration.apiTokenSecret.
agent.ts — same wiring for orchestrator.fn (declared after LinearIntegration so done in the stack rather than as a prop).
Both grants verified at synth: LinearIntegrationWebhookProcessorFn and TaskOrchestratorOrchestratorFn both carry the env var and IAM policy.

Test plan

Unit tests — 1240 baseline → 1268 CDK, 526 → 532 agent, 196 CLI, all green:

cdk/test/handlers/shared/linear-feedback.test.ts (new, 13 tests) — mutation shape, auth header, error swallowing in 4 distinct failure modes, Promise.allSettled partial-success semantics.
cdk/test/handlers/linear-webhook-processor.test.ts — extended with a user-visible feedback describe block, 10 new tests: one assertion per rejection path + happy-path-doesn't-fire + filter-rejection-doesn't-fire (latter is intentional UX).
cdk/test/handlers/orchestrate-task-feedback.test.ts (new, 5 tests) — covers notifyLinearOnConcurrencyCap with withDurableExecution mocked.
agent/tests/test_linear_reactions.py — extended with 5 sweep tests covering scoping rules (viewer-owned bgagent emojis only; preserves human reactions even with same emoji; preserves bot reactions of other emojis; sweep failure doesn't block 👀; viewer-id cached across calls).

Lint + synth + agent quality: clean.

Smoke tested on backgroundagent-dev:

Path 2 (project not onboarded) ✅
Path 5a (guardrail block) ✅ — ❌ + comment within ~20s of label
👀-fast architecture ✅ — eyes-visible at +156ms confirmed via runtime container logs

Smoke tests not run for paths 3 (defensive, no natural reproducer) and 5b (503, no natural reproducer) — covered by unit tests. Path 6 (concurrency cap) requires 4 simultaneous tasks to reproduce; covered by unit tests.

Reviewer notes

No linear_*_msg_ts fields on TaskRecord — all per-issue state stays in Linear, accessed at runtime.
No new stream consumers — reactions/comments use direct GraphQL from agent + Lambda; doesn't touch the FanOutConsumer / TaskEventsTable 2-reader-per-shard limit (closed by feat(fanout): migrate SlackNotifyFn to FanOutConsumer subscriber (#64) #79 anyway).
Sweep is daemon-threaded — never blocks the agent pipeline. If the container terminates before the sweep finishes, the worst case is the visual-duplication bug we set out to fix (status quo from before this PR).
Auth circuit breaker is per-container — resets on container restart, doesn't persist to DynamoDB. Right tradeoff for a 5-15 minute task; revisit if container lifetimes grow much longer.

Closes the silent-drop UX gap that appeared whenever a Linear-triggered task was rejected before the agent container started — the user would apply the trigger label, see nothing happen, and have no way to know why. Reactions and progress comments are emitted by the agent container; nothing fired until that point, so all upstream rejections were invisible on the Linear side. This commit wires a best-effort GraphQL feedback path covering all six distinct rejection points: In `linear-webhook-processor.ts` (pre-`createTaskCore`): 1. Issue has no projectId → "isn't in a project" comment 2. Project not onboarded / removed → "isn't onboarded; admin can run `bgagent linear onboard-project`" comment 3. Webhook missing organization or actor → diagnostic comment 4. Linear actor has no linked platform user → "v1 only the API-token owner can submit; multi-user OAuth is on the v3 roadmap" comment 5. `createTaskCore` returns non-201 → message branched on status: guardrail/validation block surfaces the user-facing error string; 503 prompts the user to re-apply the label; other 4xx/5xx falls through to a generic message. In `orchestrate-task.ts` (post-201, in admission control): 6. User concurrency cap rejection → "concurrency limit; wait for one to finish, then re-apply the label" comment. All five processor paths and the orchestrator path call a shared helper, `reportIssueFailure(secretArn, issueId, message)`, that runs the comment and ❌ reaction in parallel via `Promise.allSettled`. The helper: - Reuses the existing 5-minute `getLinearSecret` cache from `linear-verify.ts` (no extra Secrets Manager hits on warm Lambdas). - Swallows network, auth, and GraphQL errors with WARN logs — Linear feedback is advisory and must never gate the rejection path. - Posts to Linear's hosted GraphQL endpoint; mutation shapes match `agent/src/linear_reactions.py` (`commentCreate`, `reactionCreate`). CDK plumbing: - `linear-integration.ts` — wires `LINEAR_API_TOKEN_SECRET_ARN` into the webhook processor and grants read on the existing `LinearIntegration.apiTokenSecret`. - `agent.ts` — grants the same secret to `orchestrator.fn` and populates the env var. The grant is unconditional; the orchestrator only invokes the helper when `task.channel_source === 'linear'`. The non-Linear case is a hard no-op at the call site — `notifyLinear- OnConcurrencyCap` early-returns on `channel_source !== 'linear'`, and the processor only handles Linear payloads. Slack/API/webhook tasks are unaffected. Tests (28 new; 1240 → 1268, all green): - `cdk/test/handlers/shared/linear-feedback.test.ts` (13 tests): mutation shape, auth header, error swallowing in 4 distinct failure modes (secret-resolution null, non-2xx, GraphQL `errors`, network throw), `Promise.allSettled` partial-success semantics. - `cdk/test/handlers/linear-webhook-processor.test.ts` (10 new tests in a `user-visible feedback` describe block): one assertion per rejection path + happy-path-doesn't-fire + filter-rejection-doesn't- fire (the latter is intentional UX — the processor sees many events that aren't tasks, and dropping a comment on each would be noisy). - `cdk/test/handlers/orchestrate-task-feedback.test.ts` (5 tests): new file; covers `notifyLinearOnConcurrencyCap` directly with `withDurableExecution` mocked. Asserts the linear path fires; the api/webhook/slack paths no-op; missing metadata, missing env, and undefined `channel_metadata` all no-op cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

nits Wraps the v1.1 polish theme from PR aws-samples#87. Five small additions, all agent-side or docs: State-on-start (the user-visible one): - prompt_builder._channel_prompt_addendum now instructs the agent to transition the originating Linear issue to `In Progress` (or `Todo` fallback) at agent-start, mirroring the existing `In Review` chain fired at PR-open. Closes the gap where the issue stayed at `Backlog` during real agent work — only the 👀 reaction and "🤖 Starting" comment signaled progress, while humans-using-Linear expect the state column to reflect "being worked." Skips if the issue is already in `In Progress` or any later state; doesn't loop on list_issue_statuses. Alain aws-samples#63 review nits (4 small surgical changes): - linear_reactions.py: auth-failure circuit breaker. Track consecutive 401/403s; after 3 strikes, log ERROR once and short-circuit all later _graphql calls (return None) until the container restarts. Resets on any 2xx response. Replaces the prior behaviour where revoked tokens flooded CloudWatch with WARNs and wasted Linear API quota indefinitely. - pipeline.py: declare `linear_eyes_reaction_id: str | None = None` explicitly before the try block instead of relying on `locals().get("linear_eyes_reaction_id")` in the crash handler. Functionally identical; survives refactors and reads cleanly. - config.py::resolve_linear_api_token: narrow `except Exception` to `(BotoCoreError, ClientError)` from botocore.exceptions. Switch `print()` to `shell.log("WARN", ...)` so warnings join the structured log stream the rest of the agent uses. - LINEAR_SETUP_GUIDE.md + cli/src/commands/linear.ts: stop telling users to run `bgagent linear link <code>` when auto-link fails — the code generator is a v3 feature that doesn't ship in v1, so the suggestion was misleading. Replaced with explicit admin-assisted fallback (DynamoDB put-item with steps to find workspaceId, viewerId, Cognito sub) and a clear "this command exists but is non-functional in v1" note. Tests: 532 agent + 1268 cdk + 196 cli, all green. Deployed to backgroundagent-dev. Smoke-tested 👀-on-start (156ms, agent unblocked) in the prior commit; state-on-start smoke is the next manual step. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Whitespace-only changes flagged by CI's self-mutation guard. No behaviour change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

krokoko · 2026-05-15T14:27:58Z

Code Review Summary

Great PR — the architecture is sound, test coverage is thorough (28 new tests), and the "best-effort, never gate the pipeline" philosophy is consistently applied. A few items to address before merge:

Must fix

1. Thread-safety of circuit breaker globals (agent/src/linear_reactions.py)

_consecutive_auth_failures and _auth_circuit_open are read/written from both the main thread and the daemon sweep thread. The +=1 increment and the check-then-set pattern are not atomic. Worst case: duplicate "circuit OPEN" ERROR logs or lost counter resets.

→ Add a threading.Lock around the three module-level globals.

2. notifyLinearOnConcurrencyCap inside a retriable durable execution step (cdk/src/handlers/orchestrate-task.ts)

If notifyLinearOnConcurrencyCap throws unexpectedly, the durable execution framework retries the entire admission-control step — re-running failTask and emitTaskEvent (duplicate events).

→ Wrap in a defensive try-catch:

try {
  await notifyLinearOnConcurrencyCap(task);
} catch (feedbackErr) {
  logger.warn('Linear concurrency-cap feedback failed (non-fatal)', { error: ... });
}

Should fix

3. ImportError regression in resolve_linear_api_token (agent/src/config.py)

import boto3 is inside the try block but the narrowed except (BotoCoreError, ClientError) no longer catches ImportError. If boto3 is missing from the container image, the agent hard-crashes instead of gracefully degrading.

→ Add ImportError to the except tuple, or move the imports before the try.

4. Daemon thread crash silence (agent/src/linear_reactions.py)

If _sweep_stale_reactions hits an unexpected TypeError/AttributeError, the thread silently dies with no application-level log (stderr may not reach CloudWatch in containerized environments).

→ Wrap the thread target in a top-level try-except that logs via log("ERROR", ...).

5. Sweep delete counter inaccuracy (agent/src/linear_reactions.py)

_graphql(_DELETE_MUTATION, {"id": rid}) return value is discarded but deletes += 1 always fires. The summary log gives a false impression of success when deletes actually failed.

→ if _graphql(...) is not None: deletes += 1

Nice to have (non-blocking)

Tighten buildCreateTaskFailureMessage params — the sole callsite passes APIGatewayProxyResult fields which are number and string, never undefined. The | undefined arms are dead code.
Add a log when circuit breaker short-circuits — once the circuit opens, all subsequent calls return None with zero trace. A single "circuit still open" DEBUG log aids debugging.
Log AccessDeniedException at ERROR in resolveToken — IAM misconfig is persistent, not transient. WARN severity means no alert fires for a broken deployment.
Person name in comment (config.py line 70: "per Alain's feat(linear): add Linear integration, with Slack review fixes #63 review") — prefer referencing only the issue number.

Overall: solid work. The two "must fix" items are small changes that prevent real (if unlikely) failure modes. Happy to re-review once addressed.

- linear_reactions: guard auth-circuit globals with `_auth_state_lock` so the daemon sweep thread and the main thread can't race the read-modify-write on `_consecutive_auth_failures` / `_auth_circuit_open`. - linear_reactions: wrap the daemon sweep target in `_sweep_stale_reactions_safe` so an unexpected exception logs at ERROR instead of dying silently (stderr from a daemon thread doesn't reliably reach CloudWatch). - linear_reactions: only increment the sweep delete counter when `_graphql(_DELETE_MUTATION, ...)` actually returns a non-None response — previously the summary log overstated success. - config: hoist `import boto3` out of the catch-narrowed try/except so an `ImportError` (boto3 missing from the image) degrades to a WARN log instead of crashing the agent. - orchestrate-task: wrap `notifyLinearOnConcurrencyCap` in a defensive try/catch — durable-execution retries the entire admission-control step on throw, which would re-fire `failTask` + `emitTaskEvent` and produce duplicate events. - tests: 1 new throw-propagation test for `notifyLinearOnConcurrencyCap`, 3 new tests for `resolve_linear_api_token` (cached env, no-arn, ImportError fallback). Auto-reset fixture in `test_linear_reactions.py` now also resets the circuit-breaker globals between tests so future cases don't leak state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

- linear_reactions: log a single DEBUG line when the auth circuit breaker short-circuits a call, so the path isn't zero-trace once open. - config: split the `(BotoCoreError, ClientError)` catch so `AccessDeniedException` logs at ERROR instead of WARN — IAM misconfig is persistent and should page someone, not blend into transient warnings. Also drop the personal name from the inline reference to the aws-samples#63 review. - linear-webhook-processor: tighten `buildCreateTaskFailureMessage` param types to `number` / `string` (no `| undefined`) — the only caller passes `APIGatewayProxyResult` fields which are always defined. Removes dead fallback-to-`'unknown'` branches. - test_config: 2 new tests covering the split exception path (AccessDenied → ERROR; ResourceNotFound → WARN). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

isadeks · 2026-05-15T16:15:16Z

Thanks for the thorough review @krokoko. All 9 items addressed across two commits — split must/should-fix from nice-to-have so the load-bearing changes are isolated.

Must-fix (`dd5474eb`)

Thread-safety of circuit breaker globals — added _auth_state_lock around the read-modify-write on _consecutive_auth_failures / _auth_circuit_open. The 401/403 increment-and-check is now atomic; the open-circuit check reads the flag under the lock then drops the lock before logging.
notifyLinearOnConcurrencyCap defensive try/catch — wrapped in orchestrate-task.ts. New unit test (reportIssueFailure rejection propagates (caller must catch)) asserts the helper does throw on its caller, so the surrounding catch in admission-control is load-bearing rather than redundant.
ImportError regression — split import boto3 into its own try/except ImportError block ahead of the narrowed (BotoCoreError, ClientError) catch. New test_import_error_degrades_gracefully covers it.
Daemon thread crash silence — added _sweep_stale_reactions_safe wrapper that catches Exception at the thread top-level and logs at ERROR. Thread name unchanged so existing _join_sweep_thread() test helper still finds it.
Sweep delete counter inaccuracy — deletes += 1 now gated on _graphql(_DELETE_MUTATION, ...) is not None.

Also expanded the autouse fixture in test_linear_reactions.py to reset the circuit-breaker globals between tests so future cases don't accumulate state.

Nice-to-have (`420fc113`)

DEBUG log on circuit short-circuit — linear_reactions: auth circuit still open; short-circuiting call.
AccessDeniedException → ERROR — split the catch into ClientError (inspect Error.Code, escalate AccessDeniedException to ERROR) and BotoCoreError (WARN). Two new tests cover both branches.
Tightened buildCreateTaskFailureMessage params — number, string (not | undefined) since the only caller passes APIGatewayProxyResult fields. Removed dead ?? 'unknown' fallbacks.
Person name scrub — Alain's #63 review → #63 review in config.py.

Verification

Agent: 537 tests pass (was 532 — 5 new in test_config.py)
CDK: focused suites (linear-webhook-processor, orchestrate-task-feedback, linear-feedback, orchestrate-task, create-task-core) all green; 1 new test in orchestrate-task-feedback.test.ts
Compile + ESLint + ruff format/check clean
Smoke-tested on backgroundagent-dev

Ready for re-review.

isadeks requested a review from a team as a code owner May 13, 2026 22:35

isadeks marked this pull request as draft May 13, 2026 22:47

isadeks changed the title ~~feat(linear): comment + react on pre-container task-creation failures~~ feat(linear): v1.1 polish — pre-container feedback, state-on-start, sweep, nits May 14, 2026

chore(format): apply ruff format

b047b1e

Whitespace-only changes flagged by CI's self-mutation guard. No behaviour change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

isadeks marked this pull request as ready for review May 14, 2026 21:08

isadeks and others added 3 commits May 14, 2026 14:08

Merge branch 'main' into feat/linear-processor-feedback

be93ef3

Merge branch 'main' into feat/linear-processor-feedback

b58fd6e

Merge branch 'main' into feat/linear-processor-feedback

09ace05

bgagent and others added 2 commits May 15, 2026 09:01

This was referenced May 15, 2026

feat(linear): resolve API token via AgentCore Identity (Phase 2.0a) #100

Closed

feat(linear): resolve API token via AgentCore Identity (Phase 2.0a) isadeks/sample-autonomous-cloud-coding-agents#2

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(linear): v1.1 polish — pre-container feedback, state-on-start, sweep, nits#87

feat(linear): v1.1 polish — pre-container feedback, state-on-start, sweep, nits#87
isadeks wants to merge 8 commits into
aws-samples:mainfrom
isadeks:feat/linear-processor-feedback

isadeks commented May 13, 2026 •

edited

Loading

Uh oh!

krokoko commented May 15, 2026

Uh oh!

isadeks commented May 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

isadeks commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What changed

1. Pre-container failure feedback (5 paths in processor + 1 in orchestrator)

2. State transition at agent-start

3. Stale-reaction sweep

4. PR #63 review nits (4)

Plumbing

Test plan

Reviewer notes

Uh oh!

krokoko commented May 15, 2026

Code Review Summary

Must fix

Should fix

Nice to have (non-blocking)

Uh oh!

isadeks commented May 15, 2026

Must-fix (dd5474eb)

Nice-to-have (420fc113)

Verification

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

isadeks commented May 13, 2026 •

edited

Loading

Must-fix (`dd5474eb`)

Nice-to-have (`420fc113`)