Skip to content

fix(scheduled-task): keep Agent tasks visible in Maka catalog - #5737

Merged
liugddx merged 1 commit into
apache:mainfrom
liugddx:fix/scheduled-task-tool-mode-catalog
Sep 26, 2026
Merged

liugddx merged 1 commit into
apache:mainfrom
liugddx:fix/scheduled-task-tool-mode-catalog

Conversation

@liugddx

@liugddx liugddx commented Sep 26, 2026 •

Copy link
Copy Markdown
Member

Summary

Agent-created scheduled tasks persist execution.toolMode, but the Runtime Host ScheduledTask decoder rejected that field. Catalog queries could therefore return internal_failure, leaving Maka's Scheduled tasks page empty or stale even though the task had been stored.

Preserve and validate optional toolMode across task mutations and catalog reads, including legacy records that omit it. Advance the compatibility epoch from 189 to 190 because older peers reject the field. Preserve the latest upstream time and time-zone formatting.

The ScheduledTask tool now checks the same Host catalog for the newly created ID before reporting success. If verification fails, it reports the task ID and advises querying before retrying to avoid duplicates. Agent task creation rejects only missing or whitespace-only working directories, while valid Host paths including filesystem root remain supported. Successful creation reports the stored Agent execution directory.

Verification

  • Before rebasing, verified the actual Dev UI with an isolated test profile: the weekly report appeared as an Agent scheduled task with cron 0 9 * * 5 and next run 2026-10-02 09:00 in Asia/Shanghai. The same Runtime Host query returned toolMode: direct and the expected project working directory. This verifies creation and catalog visibility; it does not verify a future scheduled firing.
  • After rebasing onto current upstream main, built @maka/core, @maka/storage, @maka/runtime, and @maka/runtime-host successfully.
  • All 35 affected tests passed:
    node --test packages/runtime-host/dist/__tests__/scheduled-task-protocol.test.js packages/runtime/dist/__tests__/scheduled-task-tools.test.js packages/core/dist/__tests__/scheduled-task.test.js
  • Added a regression test confirming a filesystem-root workspace can create an Agent scheduled task.
  • Biome checks on the changed files, git diff --check, and the protocol epoch check against upstream main passed.
  • The full repository test suite was not run.

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex assisted with diagnosis, implementation, regression tests, Dev UI verification, review response, and conflict resolution. The commit includes a Generated-by: Codex trailer.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actions github-actions Bot added the effort/M Under 500 readable lines label Sep 26, 2026

@hqhq1025 hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change preserves and validates execution.toolMode across ScheduledTask protocol mutations and catalog reads, and re-reads the Host catalog after model-created tasks. I reviewed the decoder, creation path, Host workspace resolution, and added tests. Node 24 install/build, 38 focused tests, runtime/runtime-host typechecks, Biome, and diff check passed locally. I did not reproduce the Desktop UI flow or packaged Host behavior.

P2: See the inline finding about valid filesystem-root sessions now being unable to create Agent/resume tasks. The PR also currently conflicts with main in packages/runtime-host/src/protocol/index.ts (main epoch 189 versus this branch 178) and packages/runtime/src/scheduled-task-tools.ts (main added Host clock/time-zone output); resolve both and revalidate the merged behavior. Only the label check is currently reported on this head, so merge readiness is not established.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

Comment thread packages/runtime/src/scheduled-task-tools.ts Outdated
@liugddx
liugddx force-pushed the fix/scheduled-task-tool-mode-catalog branch from 4da07a2 to 75838a4 Compare September 26, 2026 08:28

@hqhq1025 hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On this rebased head, the ScheduledTask decoder preserves and validates Agent toolMode, and the model tool verifies a created task by reading the Host catalog. The compatibility epoch advances from main 189 to 190, and the mainline Host clock/time-zone output remains intact. I reviewed the full five-file diff, protocol/query and Host workspace paths, plus the new tests. Node 24 install/build, 41 focused tests, runtime/runtime-host typechecks, Biome, and diff check passed; current-head CI test is green and the branch is mergeable.

P2 remains in the inline finding: a valid filesystem-root Host session can no longer create resume or Agent scheduled tasks. This is not ready to merge until that policy mismatch is resolved. I did not independently run the Desktop catalog UI or packaged Host end-to-end.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

Comment thread packages/runtime/src/scheduled-task-tools.ts Outdated
@liugddx
liugddx force-pushed the fix/scheduled-task-tool-mode-catalog branch from 75838a4 to 7d29b0e Compare September 26, 2026 09:20

@hqhq1025 hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed the five-file diff on this head, including the new change to the ScheduledTask model-tool cwd guard, protocol read/write handling for Agent toolMode, Host catalog verification, and their tests. The previous P2 is addressed: the tool now rejects only missing cwd for non-notification effects, so an existing filesystem-root Host workspace can reach task creation; the new root-workspace regression test passes. The compatibility epoch is 190, and the Host clock/time-zone output from main remains intact. I found no further substantiated P0–P3 issue in the reviewed paths.

Node 24 build, 40 focused tests, runtime typecheck, Biome, diff check, and merge-tree passed locally. The current-head CI test is green and GitHub reports the PR mergeable. I did not independently run the Desktop catalog UI or a packaged Host end-to-end; a Host catalog read alone does not prove a Desktop connected to a different Host rendered the task. Final merge decision remains with a human reviewer.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

@liugddx

liugddx commented Sep 26, 2026

Copy link
Copy Markdown
Member Author
image image

@likun666661

Copy link
Copy Markdown
Member

Non-blocking review note on the creation confirmation: buildScheduledTaskTool calls authority.list() after creation and finds the new ID, but HostScheduledTaskCoordinator.list() reads the store directly. The Desktop Scheduled tasks page uses scheduled-task.query and its protocol decoder. Thus this readback confirms that the task is present in the Host store; by itself it does not verify that the Desktop protocol query succeeds or that the page renders it. The decoder change and its tests address the reported failure, and the PR describes a separate Dev UI check before the rebase. I would narrow the success wording/PR claim to the actual readback guarantee, or add a final-head protocol/UI check if the intended claim is end-to-end visibility. This is a non-blocking verification-boundary suggestion, not a P1/P2 finding on the current head.

@likun666661 likun666661 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved on head 7d29b0e. The protocol decoder now preserves and validates optional Agent toolMode across mutation and catalog read paths, with legacy omission covered; the earlier root-workspace regression is addressed. No substantiated P1/P2 blocker remains in the reviewed diff. My non-blocking note on the scope of the post-create store readback is in the PR conversation; this approval does not claim an independent final-head Desktop E2E or a scheduled firing test.

@liugddx
liugddx merged commit 860ff50 into apache:main Sep 26, 2026
1 check passed
Shouly pushed a commit to Shouly/maka that referenced this pull request Sep 27, 2026
Watermark bfb315a. Done: apache#5573/apache#5600/apache#5601, apache#5521, apache#4875, apache#5723, apache#5738,
apache#5742. Not applicable: apache#5737, apache#5593. Deferred: apache#5730. Consider: apache#5599,
apache#5120, apache#5693. Diverged: apache#5740. Skipped: ACP, WorkHub, upstream renderer and
packages/ui, one refactor.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants