fix(tools): surface taskId on subagent failure paths in task tool

[ausard]

Closes #886.

Summary

Mirrors kilocode #11621 (merge 4a8f324, 2026-06-30). The task tool
used to swallow the task id whenever a subagent failed BEFORE producing
visible output, leaving the parent unable to call task_status to
recover. This change surfaces taskId on the validate-failure return
and verifies the background .catch round-trip via the store.

Changes

src/tool/tools/task.ts

• Pre-bind taskIdForFailure = params.task_id ?? nanoid() BEFORE the
  TaskTool.validate call so the id exists from the start of the
  function.
• The validate-failure return at the old :202-205 now carries
  data: { taskId, agentId, response: '', completed: false, status: 'failed' }
  in addition to success: false and error.
• Reuse taskIdForFailure as the id of the freshly-inserted store row,
  so the id is stable across the validate boundary.
• Tool description updated to mention that task_id can also be used to
  inspect a failed task.

src/tool/tools/task_status.ts – no change needed. Already surfaces
task.error and task.status (verified via new round-trip test).

tests/tool/tools/task-failure-paths.test.ts – new file with three
scenarios:

1. Validate-failure path produces a non-empty data.taskId and
   status: 'failed' (uses a vi.mock on ../../src/agent/index.js
   to force a primary-mode explore agent).
2. Caller-supplied task_id is preserved through validate-failure.
3. Background .catch round-trip: simulate the store mutation that the
   background .catch performs, then assert task_status surfaces
   both status: 'failed' and error.

Verification

npm run lint            # 0 errors
npm run typecheck       # clean
npm run format:check    # clean
npm test                # 2804 passing
npm run build           # clean

Coverage: 62.36% lines (well above the 40% CI floor).

Source

Kilo-Org/kilocode#11621 — +96 -4 in
packages/opencode/src/tool/task.ts. Research brief:
.github/research/2026-06-30-research.md (Detailed Finding #3).

[alexi-bot]

[github-actions]

Coverage Report

Metric	Coverage
Lines	62.3%
Statements	62.3%
Functions	57.71%
Branches	57.05%

Coverage Details

• Lines: 7656/12288
• Statements: 7964/12783
• Functions: 1186/2055
• Branches: 4315/7563

[ausard]

Summary

PR #889 (fix(tools): surface taskId on subagent failure paths in task tool)
is a small, focused tool-internals fix: two files changed, +171 / -5, no
new imports, no new dependencies, no new top-level directories, no
routing-config or provider-interface changes.

From an Architecture-vertical perspective this PR is clean: it does
not introduce any new structural violation and does not require an ADR.
The behavioural improvement (returning a stable taskId on validation
failure so the parent can call task_status on it) is local to
src/tool/tools/task.ts and respects existing layering.

The findings below are the standing F1-F14 backlog from
docs/adr/REVIEW-2026-06-29.md. They are not introduced by this PR
and must not block it -- but per the review cadence I re-verify their
status at the PR HEAD (48658c13) for the record.

Re-verification result: every block-severity finding from the
2026-06-29 review is still present, unchanged, at this HEAD. PR #889
itself is approved-from-architecture.

Findings

F0 -- info -- PR #889 scope vs. architecture surface (NEW, this PR)

Scope check on the diff:

$ git diff origin/master...HEAD --stat
 src/tool/tools/task.ts                      |  26 ++++-
 tests/tool/tools/task-failure-paths.test.ts | 150 ++++++++++++++++++++++++++++
 2 files changed, 171 insertions(+), 5 deletions(-)

$ git diff origin/master...HEAD --name-only | \
    grep -E "routing-config\.json|src/providers/index\.ts|^src/[^/]+$|^src/[^/]+/index\.ts"
(no output)

$ git diff origin/master...HEAD -- src/tool/tools/task.ts | \
    grep -E "^[+-]\s*(import|from)\b"
(no output)

No new top-level src/ directory, no routing-config.json change, no
provider-interface change, no new imports. task.ts continues to import
only from ../index.js (the tool registry), ../../agent/index.js
(known cross-layer, see F4), and ../../core/costTracker.js (downward,
allowed). The new error-path branch reuses nanoid() which is already a
transitive dep of the file -- no new dependency.

Severity info. No action needed for PR #889 itself.

F1 -- block -- ADRs 002..N still missing (carried)

ls docs/adr/ at this HEAD:

$ ls docs/adr/
001-establish-adr-process.md
REVIEW-2026-06-04.md
REVIEW-2026-06-04T12-followup.md
REVIEW-2026-06-15.md
REVIEW-2026-06-29.md

Top-level src/ directories at this HEAD: 35 (ls -d src/*/ | wc -l).
ADR 001 declared backfill required. Still only one ADR on disk. Deficit
of 34 unjustified top-level dirs (or 31 if Recommendation 1 of the
2026-06-29 review lands first).

F2 -- block -- src/cli/commands/models.ts direct SDK import (carried)

$ grep -rEn "from\s+['\"]@sap-ai-sdk" src/ --include='*.ts' --include='*.tsx' \
    | grep -v "^src/providers/"
src/cli/commands/models.ts:6:import { DeploymentApi } from '@sap-ai-sdk/ai-api';

Constitution III violation, unchanged for three consecutive reviews.

F3 / F3b -- warn -- provider-surface callers not migrated (carried, half-fixed)

src/providers/index.ts re-exports CompletionResult and TokenUsage
(verified). Callers still bypass:

$ grep -n "from '../providers/sapOrchestration\|from '../providers/sessionHeaders" \
    src/core/*.ts | grep -v __tests__
src/core/agenticChat.ts:19:import type { CompletionResult, TokenUsage } from '../providers/sapOrchestration.js';
src/core/streamingOrchestrator.ts:16:import { buildSessionHeaders } from '../providers/sessionHeaders.js';

$ grep -n "buildSessionHeaders" src/providers/index.ts
(no output -- not re-exported)

So F3b (buildSessionHeaders missing from providers/index.ts) is
strictly worse than F3: the consumer cannot migrate until the
provider-surface re-export is added. One-line addition.

F4 -- warn -- six src/tool/ -> src/agent/ upward imports (carried)

$ grep -rEn "from\s+['\"][^'\"]*\.\./agent/[^'\"]*['\"]" src/tool/
src/tool/tools/edit.ts:9:      ../../agent/agentsMdReminders.js
src/tool/tools/read.ts:19:     ../../agent/agentsMdReminders.js
src/tool/tools/multiedit.ts:9: ../../agent/agentsMdReminders.js
src/tool/tools/write.ts:11:    ../../agent/agentsMdReminders.js
src/tool/tools/grep.ts:19:     ../../agent/agentsMdReminders.js  (attachAgentsMdRemindersForPaths)
src/tool/tools/task.ts:7:      ../../agent/index.js              (getAgentRegistry, Agent)

PR #889 modifies src/tool/tools/task.ts but does not add any new
upward import; line 7 was already present on master and remains
unchanged. The existing F4 violation is not aggravated by this PR.

F5 -- warn -- Kotlin files still in src/permission/ (carried, 3rd review)

$ ls src/permission/PermissionView.kt src/permission/PermissionViewTest.kt
src/permission/PermissionView.kt
src/permission/PermissionViewTest.kt

Cannot be transpiled, type-checked, or executed by Node. One-line
git rm PR.

F6 -- warn -- src/core/migration/ SQL artefacts (carried)

Still present. No better-sqlite3 / Prisma dep added in the interim,
no new migrations introduced this cycle.

F7 -- info -- role-architecture.md module map drift (carried)

The module map at the top of this very prompt still lists ~10 modules.
Reality is 35. Replace inline list with a pointer to
docs/ARCHITECTURE.md. Recommendation carried verbatim from
REVIEW-2026-06-29.

F8 -- info -- ESM .js import discipline (re-verified clean)

$ grep -rEn "^\s*(import|export)\b.*from\s+['\"]\.\.?/[^'\"]+['\"]" src/ \
    --include='*.ts' --include='*.tsx' \
  | grep -vE "from\s+['\"]\.\.?/[^'\"]+\.js['\"]" \
  | grep -vE "from\s+['\"]\.\.?/[^'\"]+\.json['\"]"
src/context/__tests__/ranking.test.ts:54: import a from './a';
src/context/__tests__/ranking.test.ts:55: import b from './b';

Both are inside JavaScript-source-code string literals used as test
fixtures by the ranking unit tests -- they are data, not real imports.
Verified by reading the file: these lines sit inside a
` template-literal block passed to the importer ranker. No real
violations.

F9 -- info -- one test-file cross-layer import (carried)

src/plugin/__tests__/ruleCommand.test.ts:47 still imports
buildAssembledSystemPrompt from ../../agent/system.js. Resolution
remains "codify __tests__/ slack in ADR 002".

F10 / F11 / F12 -- block -- orphan files still in src/core/ (carried)

$ ls src/core/billing.ts src/core/catalog.ts src/core/integration/schema.ts
src/core/billing.ts
src/core/catalog.ts
src/core/integration/schema.ts

Unchanged from 2026-06-29. Eligible for the combined cleanup PR.

F13 -- block -- three empty placeholder top-level dirs (carried)

$ ls -d src/router src/session src/ui
src/router
src/session
src/ui

$ wc -l src/router/index.ts src/session/data.ts src/ui/timers.ts
1 src/router/index.ts
1 src/session/data.ts
1 src/ui/timers.ts

$ grep -rEn "from ['\"]\.\.?/(\.\.?/)*router/" src/ --include='*.ts' --include='*.tsx'
$ grep -rEn "from ['\"]\.\.?/(\.\.?/)*session/data" src/ --include='*.ts' --include='*.tsx'
$ grep -rEn "from ['\"]\.\.?/(\.\.?/)*ui/timers" src/ --include='*.ts' --include='*.tsx'
(no output for any of the three)

Direct violation of ADR 001 (new top-level src/<name>/ requires an
ADR before merge). Still zero consumers. Eligible for the combined
cleanup PR.

F14 -- info -- sync-allowlist guardrail still unfiled (carried)

infrastructure vertical deliverable. Architecture-vertical view: the
absence of this guardrail is what allows F10..F13 to keep recurring.

Recommendations

PR #889 specific:

1. Merge PR fix(tools): surface taskId on subagent failure paths in task tool #889 from an architecture perspective. Zero structural
   impact, zero new violations, no ADR required. (Quality vertical
   should still verify the unit-test additions and the new failure-path
   data shape against the tool-result contract.)

Carried, unchanged from REVIEW-2026-06-29 (none caused by this PR,
none blocking this PR):

2. Land the combined cleanup PR (chore(sync)) that removes
   src/core/billing.ts, src/core/catalog.ts,
   src/core/integration/, and src/router/ src/session/ src/ui/.
   Closes F10, F11, F12, F13 in one diff.
3. Land the sync-allowlist guardrail (infrastructure vertical) so the
   next sync cycle does not re-add F13-shaped placeholders.
4. Finish the F2/F3/F3b migration: add buildSessionHeaders (and a
   listDeployments helper) to src/providers/index.ts and rewrite
   the three caller imports.
5. git rm the two Kotlin files (F5).
6. Decide F6 -- either own the migrations directory with a real
   loader + dep, or remove it.
7. Update .github/prompts/role-architecture.md to point at
   docs/ARCHITECTURE.md for the module map (F7).
8. File ADR 002 (baseline backfill) after Recommendation 2 lands.

ADR needed?

No. PR #889 does not change layering, does not add a top-level
src/ directory, does not modify the provider interface, and does not
change routing-config.json. None of the ADR 001 triggers fire.

The standing ADR 002..005 backfill obligation from REVIEW-2026-06-29
is unchanged -- and explicitly not a prerequisite for merging
PR #889.

[ausard]

Security review (role-security, automated)

Scope: PR #889 (auto/886-tool-surface-taskid-on-subagent-failure-paths-in-t -> master),
2 commits, 3 files changed: src/tool/tools/task.ts,
tests/tool/tools/task-failure-paths.test.ts,
.github/workflows/documentation-update.yml.

Summary

No block-severity findings in this PR diff. Lockfile is untouched
(package-lock.json not in the diff), so no new transitive deps were
added and the known repo-wide npm audit baseline is unchanged by this
PR. marked@>=16 is NOT introduced (still pinned ^15.0.12). No secret
material appears in the diff. No permission-gate weakening, no new
auto-approve surface, no pull_request_target exposure. The workflow
change adds a retry loop around npm install -g @kilocode/cli and
npm ci in a dedicated install step - it does NOT fall into the
AGENTS.md "embedded inside run_check" trap.

Two info-level observations are worth a follow-up, but neither blocks
merge.

Critical (block)

None.

High (warn)

None introduced by this PR.

(Repo-wide audit baseline shown under Info for visibility; pre-existing
and out of scope for this PR.)

Info

i1. Echo-back of unvalidated task_id on the validate-failure path
(src/tool/tools/task.ts:201, :211).

The new taskIdForFailure = params.task_id ?? nanoid() reuses the
caller-supplied task_id verbatim and now echoes it back in
result.data.taskId on the validate-failure return. task_id is
typed z.string().optional() with no length cap or charset constraint
(TaskParamsSchema, line 14). For a sub-agent boundary the previous
behaviour silently dropped the id; the new behaviour reflects whatever
the parent provided. Risk is low (the value flows back to the same
agent that supplied it), but a parent prompt-injected into setting
task_id to a multi-MB string or to control characters would now see
that string reflected in its own tool output, expanding the
prompt-injection self-loop surface slightly. Suggestion: tighten the
schema to z.string().min(1).max(128).regex(/^[A-Za-z0-9_-]+$/) or
explicitly call nanoid() whenever the caller-supplied id fails that
shape. Not blocking for this PR.

i2. Subagent permission inheritance still TODO
(src/tool/tools/task.ts:246-258).

Pre-existing - NOT introduced by this PR - but called out because the
PR widens the contract for failure paths and the next iteration of
this tool will almost certainly touch the same area:
subagentConfig is built with autoMode: false hardcoded and the
deriveSubagentSessionPermission helper is left commented out. Until
that is wired, a subagent inherits the parent's full permission set
implicitly via process-level state rather than via the documented
restriction-derivation path. Track in a follow-up issue with the
security label.

i3. Repo-wide npm audit --omit=dev baseline (out of scope for
PR #889; surface for visibility).

Lockfile unchanged in this PR, so this baseline is unchanged by the
PR. Snapshot at review time: 11 advisories, 8 high, 3 moderate, 0
critical. Notable items:

• xlsx@^0.18.5 (direct dep) - high; fixAvailable: false on the
  npm-registry copy (real fix only on cdn.sheetjs.com).
  GHSA-4r6h-8v6p-xvw6 (proto pollution) + GHSA-5pgg-2g8v-p4x9 (ReDoS).
• axios transitive - 4 advisories (proto pollution, NO_PROXY bypass,
  cookie ReDoS) - fixAvailable: true.
• @hono/node-server, @xmldom/xmldom, express-rate-limit,
  fast-uri, form-data, path-to-regexp, js-yaml, qs,
  ip-address - all transitive, all fixAvailable: true.

Out of scope for this PR; track separately. Do NOT run
npm audit fix --force blindly per AGENTS.md.

Recommendations

1. (optional, this PR) Tighten TaskParamsSchema.task_id to a bounded
   id pattern, OR fall back to nanoid() whenever the caller-supplied
   value fails a shape check. Single-line change in
   src/tool/tools/task.ts:201.
2. (separate issue, security label) Wire
   deriveSubagentSessionPermission end-to-end in taskTool.execute
   so subagents inherit restrictions explicitly, not implicitly. The
   TODO at src/tool/tools/task.ts:246-258 already maps the work.
3. (separate issue, security label) Plan the xlsx migration -
   either pin to the SheetJS-CDN tarball with package.json overrides,
   or move the importer behind a feature flag. Until then, document
   that xlsx-parsed input is treated as untrusted.

Gate status (informational)

• Secret leakage in diff: clean.
• marked@>=16 introduced: no (still ^15.0.12).
• Lockfile drift outside npm commands: no (lockfile unchanged).
• Permission system / --dangerously-skip-permissions: untouched.
• Workflow secret-logging / pull_request_target exposure: clean.
• npm install -g embedded in run_check: no (own dedicated step).

-- role-security, generated 2026-06-30

[ausard]

Architecture Review -- PR #889

• Trigger: pull_request on refs/pull/889/merge
• PR: fix(tools): surface taskId on subagent failure paths in task tool #889 fix(tools): surface taskId on subagent failure paths in task tool
• Base: master -- Head: auto/886-tool-surface-taskid-on-subagent-failure-paths-in-t
• HEAD: 9a902af9
• Reviewer: role-architecture (Alexi T-shape factory)

Summary

PR #889 is a narrow fix(tools) change. It touches three files:

.github/workflows/documentation-update.yml  | retry-on-network-error tweak
src/tool/tools/task.ts                      | bind taskId before validate
tests/tool/tools/task-failure-paths.test.ts | new test, 150 lines

The structural footprint is zero: no new top-level src/ directory, no
new ADR-requiring boundary change, no new import edges across layers. The
diff adds no imports at all to src/tool/tools/task.ts (the pre-existing
import of getAgentRegistry, type Agent from ../../agent/index.js was
already in master and is finding F4 carried in every architecture
review since REVIEW-2026-06-15).

Verdict for this PR: no architectural concerns introduced. Approve from
the architecture vertical. The change is exactly the kind of small,
contained, well-tested fix the role-engineering vertical owns end to end.

However, the broader repository state at this HEAD is essentially
identical to the state captured in docs/adr/REVIEW-2026-06-29.md one
day ago. All block-severity findings from that review are still open:

ID	Path	First flagged
F2	src/cli/commands/models.ts:6 direct @sap-ai-sdk/ai-api import	REVIEW-2026-06-04
F10	src/core/billing.ts (10 lines, no callers)	REVIEW-2026-06-15
F11	src/core/catalog.ts (152 lines, no callers, name collision with core/router.ts)	REVIEW-2026-06-15
F12	src/core/integration/schema.ts (single-line comment)	REVIEW-2026-06-15
F13	src/router/, src/session/, src/ui/ -- three placeholder top-level dirs, no ADR	REVIEW-2026-06-29

ls -1 src/ | wc -l is still 35 top-level dirs. The role map in
.github/prompts/role-architecture.md still lists ~10. The drift
documented as F7 has not closed.

This PR does not make any of those worse. None of the recommended
clean-up PRs from REVIEW-2026-06-29 have landed yet -- this is not
that PR.

Findings

A1 -- info -- PR #889 introduces no structural change

Diff inventory:

$ git diff origin/master...HEAD --stat
 .github/workflows/documentation-update.yml  |  35 ++++++-
 src/tool/tools/task.ts                      |  26 ++++-
 tests/tool/tools/task-failure-paths.test.ts | 150 ++++++++++++++++++++++++++++

$ git diff origin/master...HEAD -- src/tool/tools/task.ts | grep -E "^[+-]import"
(no output)

• No new files under src/.
• No new imports added or removed in production code.
• The behavioural change is to pre-bind taskIdForFailure = params.task_id ?? nanoid()
  in src/tool/tools/task.ts:199 so the validate-failure return at
  task.ts:204-217 carries a data payload with taskId. The
  resume-or-create block is rewritten to reuse that same id when the
  task is fresh and the caller-supplied id when resuming. This is a
  local invariant change inside one function in one file.

Architecture impact: none. The provider boundary, tool registry pattern,
and layering are untouched.

A2 -- info -- F4 (tool -> agent inversion) is exercised but not introduced

src/tool/tools/task.ts:7 already imports getAgentRegistry, type Agent
from ../../agent/index.js. That import is unchanged in this PR. The
underlying inversion is F4 in REVIEW-2026-06-04.md /
REVIEW-2026-06-15.md / REVIEW-2026-06-29.md and is in the queue for
ADR 002 (codify the agent-registry slack) per recommendation 6 of
REVIEW-2026-06-15.

Action for this PR: none. Touching the import here would expand
scope; the resolution belongs to a dedicated refactor(tools) PR.

A3 -- info -- ESM .js import discipline remains clean

$ grep -rEn "from ['\"]\.\.?/[^'\"]+['\"]" src/ \
    --include='*.ts' --include='*.tsx' | \
    grep -vE "\.js['\"]|\.json['\"]|\.css['\"]"
src/context/__tests__/ranking.test.ts:54:import a from './a';
src/context/__tests__/ranking.test.ts:55:import b from './b';

Both hits are inside JavaScript template literals used as test fixtures
in src/context/__tests__/ranking.test.ts (the file analyses source
strings, it does not actually import from ./a or ./b). No real
bare imports anywhere. F8 remains green.

A4 -- info -- Cross-package import audit unchanged from REVIEW-2026-06-29

Lower-layer -> upper-layer scan run on HEAD 9a902af9:

# src/tool/ -> src/cli/ or src/agent/
src/tool/tools/edit.ts:9       attachAgentsMdReminders from '../../agent/agentsMdReminders.js'
src/tool/tools/read.ts:19      attachAgentsMdReminders from '../../agent/agentsMdReminders.js'
src/tool/tools/multiedit.ts:9  attachAgentsMdReminders from '../../agent/agentsMdReminders.js'
src/tool/tools/write.ts:11     attachAgentsMdReminders from '../../agent/agentsMdReminders.js'
src/tool/tools/grep.ts:19      attachAgentsMdRemindersForPaths from '../../agent/agentsMdReminders.js'
src/tool/tools/task.ts:7       getAgentRegistry, type Agent from '../../agent/index.js'

# src/providers/ -> src/core/ or src/cli/
(no output)

# src/core/ direct provider SDK import
src/cli/commands/models.ts:6   DeploymentApi from '@sap-ai-sdk/ai-api'   # F2, in cli/ not core/

# src/{bus,permission,mcp,hooks,plugin,skill}/ -> src/cli/ or src/agent/
src/plugin/__tests__/ruleCommand.test.ts:47   buildAssembledSystemPrompt from '../../agent/system.js'   # test-only, F9

Exactly the same set as REVIEW-2026-06-29. F4 (six tool -> agent
inversions) and F9 (one plugin-test -> agent inversion) carried forward
unchanged; F2 carried forward unchanged.

A5 -- warn -- F10 / F11 / F12 / F13 still open at PR HEAD

Re-verified one day after REVIEW-2026-06-29:

$ wc -l src/router/index.ts src/session/data.ts src/ui/timers.ts \
        src/core/billing.ts src/core/catalog.ts src/core/integration/schema.ts
   1 src/router/index.ts
   1 src/session/data.ts
   1 src/ui/timers.ts
  10 src/core/billing.ts
 152 src/core/catalog.ts
   1 src/core/integration/schema.ts
 166 total

$ grep -rln "core/billing\|core/catalog\|core/integration" src/ \
    --include='*.ts' --include='*.tsx'
(no output -- still no callers)

This PR does not need to fix them, but the cleanup PR proposed in
Recommendation 1 of REVIEW-2026-06-29 has not yet been opened.

A6 -- warn -- F5 (.kt files in src/permission/) still present

$ ls src/permission/PermissionView.kt src/permission/PermissionViewTest.kt
src/permission/PermissionView.kt
src/permission/PermissionViewTest.kt

Now flagged in four consecutive reviews. They are not TypeScript,
cannot be type-checked, cannot run. Severity remains warn only by
convention; in practice this is dead foreign-source bytes that ship in
every commit.

A7 -- info -- F6 (src/core/migration/) unchanged

Still five SQL artefacts in src/core/migration/ (three loose files +
two Prisma-style directories). No better-sqlite3 / prisma
dependency in package.json. No new migrations this cycle.

Recommendations

Targeted at this PR first, repo-wide second.

1. Merge PR fix(tools): surface taskId on subagent failure paths in task tool #889 from the architecture perspective. The structural
   surface is unchanged. The fix is localised, well-tested
   (tests/tool/tools/task-failure-paths.test.ts, +150 lines), and the
   pattern of pre-bind id -> attach to all failure-path data payloads
   is a sound invariant. role-quality should drive the merge decision;
   role-architecture has no objection.

2. Open the long-overdue cleanup PR (chore(sync)). Same
   recommendation as REVIEW-2026-06-29 refactor: modularize CLI structure and simplify providers #1, repeated because it has not
   landed and was not addressed in fix(tools): surface taskId on subagent failure paths in task tool #889:

   git rm src/core/billing.ts                  # F10
   git rm src/core/catalog.ts                  # F11
   git rm -r src/core/integration              # F12
   git rm -r src/router src/session src/ui     # F13
   git rm src/permission/PermissionView.kt     # F5
   git rm src/permission/PermissionViewTest.kt # F5

   Then npm run typecheck && npm run lint && npm run build. Owner:
   role-engineering. This is the single highest-leverage commit
   available to this repo.

3. Sync-allowlist guardrail in the sync workflow. Re-stated from
   REVIEW-2026-06-29 feat(cli): add Homebrew tap support #2; the role-infrastructure deliverable that
   prevents F10..F14 from recurring. Without it, the next
   feat(sync): apply upstream changes PR is likely to add a fourth
   placeholder top-level dir.

4. Replace the inline module map in .github/prompts/role-architecture.md
   with a pointer to docs/ARCHITECTURE.md. The role file still
   advertises ~10 top-level modules; reality is 35. The map will keep
   drifting unless its source of truth is moved to one place. Owner:
   role-architecture, single-line change. Defer if the cleanup PR
   above lands first (it will change the count) -- otherwise file
   immediately.

ADR needed?

No new ADR for PR #889. The change is a behavioural fix inside one
existing tool file; no boundary, no schema, no new directory.

ADR debt for the repository is unchanged from REVIEW-2026-06-29 and
remains the responsibility of the next architecture-vertical cycle:

• ADR 002 (test-file slack for cross-layer imports) -- pending.
• ADR backfill 003..N for the ~17-now-20 top-level dirs created without
  an ADR -- pending.

Neither is blocked by, nor in scope of, this PR.

[ausard]

Review - role-quality

• typecheck
• lint (0 errors; 1285 pre-existing warnings, none new)
• format
• tests (3 new tests, all passing; 14/14 across the two task test files)
• build
• coverage: 62.40% lines (>= 40% floor); src/tool/tools/task.ts now at 93.75% statements

Scope verified

git diff master...HEAD shows 3 files, 203 / 8 lines:

• src/tool/tools/task.ts — pre-binds taskIdForFailure (caller id or fresh nanoid()) and returns a populated data: { taskId, agentId, response: '', completed: false, status: 'failed' } block on the TaskTool.validate rejection path. This is the kilocode #11621 parity fix.
• tests/tool/tools/task-failure-paths.test.ts — 3 deterministic tests covering: validate-failure surfaces taskId; caller-supplied task_id is reused on validate-failure; task_status surfaces status: 'failed' + error from a background .catch mutation.
• .github/workflows/documentation-update.yml — retry-with-backoff wrappers around npm install -g @kilocode/cli and npm ci, with the two install steps cleanly separated (previously the first step also ran npm ci, which was the trap called out in AGENTS.md).

Correctness

The Resume or create task branch reads correctly:

• caller passed task_id AND store has it → resume (line 237 reassigns taskId = params.task_id which is safe since taskData truthiness gates the branch).
• caller passed task_id AND store does NOT have it → create new row keyed by that same id (id stable across validate-failure and live execution).
• no task_id → mint with nanoid(), store under that.

Test mocks the registry BEFORE the tool import (vi.mock hoist + explicit ordering, per AGENTS.md). Temp state is cleared in afterEach via getTaskStore().clear() and delete process.env.ALEXI_EXPERIMENTAL_BACKGROUND_TASKS — parallel-safe.

Security / style

• No new dependencies. No any, no @ts-ignore. One as string cast on line 237 is justified (branch is gated by taskData existing, which requires params.task_id).
• All local imports use .js extensions. Zod schema unchanged.
• No secrets, no eval, no prototype pollution surface.
• The workflow retry loops use the same shape as agent-autohealing.yml / auto-implement.yml — consistent with the factory pattern.

Minor observations (non-blocking)

• The taskStatus data shape on the validate-failure return includes response: '' — fine, but a future caller may want to surface the validator error message in response too. Not in scope here.
• Background path .catch at line 284-288 still uses fire-and-forget; the test exercises store mutation directly, which matches the contract task_status is supposed to expose. A real end-to-end test for the .catch path would need the queueBackgroundTask stub to be wired up, which is out of scope.

Changes made: none.

Verdict: Approved