fix(backend): stop routine tasks from getting office skills they can't run - #3502
Conversation
…ime env A heavy-routine task launches on the kanban path (IsFromOffice=false by design), which never sets KANDEV_CLI or the other office runtime vars — but skill deploy is keyed on the agent profile, not on office-ness, so the routine agent's worktree still got the bundled office skills (kandev-protocol, kandev-task-ops, ...) whose instructions all assume those vars are set. Every command they told the agent to run failed. Gate deployment of is_system skills on the launch's finalized env actually carrying KANDEV_CLI (skill.Request/SkillDeployRequest.OfficeRuntime), derived once in runSkillDeploy from the finalized launch env. User-authored skills are unaffected.
GitHub Actions
Inactive
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: QUIET Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: QUIET Plan: Advanced Run ID: 📒 Files selected for processing (9)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 SummarySummary by CodeRabbit
WalkthroughThe deployment flow derives Office runtime availability from ChangesOffice runtime skill gating
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to System Office skills are now omitted when the Office CLI runtime is unavailable, while user-authored skills remain deployed. The runtime signal and manifest behavior are covered by targeted tests, with no concrete current-head merge-blocking risk identified. Sequence Diagram(s)sequenceDiagram
participant PreparedLaunch
participant runSkillDeploy
participant skillDeployerAdapter
participant RuntimeDeployer
participant ManifestBuilder
PreparedLaunch->>runSkillDeploy: Read KANDEV_CLI
runSkillDeploy->>skillDeployerAdapter: Set OfficeRuntime
skillDeployerAdapter->>RuntimeDeployer: Forward OfficeRuntime
RuntimeDeployer->>ManifestBuilder: Build manifest with OfficeRuntime
ManifestBuilder->>ManifestBuilder: Skip system skills when false
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit checks the launch-time air Comment |
|
| Filename | Overview |
|---|---|
| apps/backend/internal/agent/runtime/lifecycle/skill_deploy.go | Derives the new deployment gate from only KANDEV_CLI, allowing incomplete non-Office environments to pass. |
| apps/backend/internal/agent/runtime/lifecycle/skill/manifest.go | Filters classified system skills from manifests when the supplied Office runtime signal is false. |
| apps/backend/internal/agent/runtime/lifecycle/skill/types.go | Adds the system-skill classification to the runtime skill model. |
| apps/backend/internal/office/skills/runtime_adapter.go | Correctly propagates the persisted IsSystem classification into the runtime model. |
| apps/backend/internal/agent/runtime/lifecycle/skill/deployer_officeruntime_test.go | Covers manifest delivery for system and user-authored skills in both gate states, but assumes the boolean itself represents a complete Office environment. |
| apps/backend/internal/agent/runtime/lifecycle/skill_deploy_test.go | Covers derivation from presence or absence of KANDEV_CLI but does not exercise an incomplete environment containing only that variable. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Finalized launch environment] --> B{KANDEV_CLI non-empty?}
B -- No --> C[Omit system skills]
B -- Yes --> D[Deploy system skills]
D --> E[Skills invoke Office CLI]
E --> F{API credentials and runtime identity present?}
F -- Yes --> G[Office operations run]
F -- No --> H[CLI operations fail]
Reviews (1): Last reviewed commit: "fix(backend): stop deploying office syst..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Claude finished @nova28's task in 5m 31s —— View job FindingsSuggestion (recommended, doesn't block)
Summary
Verdict: Ready to merge with suggestions. The core mechanism is sound: |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
GitHub Actions
Inactive
|
Thanks for the contribution. We pushed |
GitHub Actions
Inactive
|
CI fix: updated the launch-context regression to create and assign a real Office task. This keeps the test aligned with the Office-only runtime gate and prevents normal Kanban tasks from being treated as Office sessions. |
2 participants
Tip
PR walkthrough: Open the visual walkthrough
Heavy Office routine tasks launch through the same kanban path as an ordinary task rather than the Office scheduler path, so they never receive
KANDEV_CLIin their environment — yet they still received the bundled Office skills, every one of which shells out to$KANDEV_CLI. This change stops deploying those system skills to a launch whose finalized environment doesn't actually carry Office runtime env.Today: A heavy (task_template) Office routine's agent still gets the bundled
kandev-task-opsandkandev-approvalsskills, because they're written per-workspace and were never gated on how the task launched. The first office-skill command the agent tries expands to a barekandev ...with no CLI on the PATH and fails.After this: Bundled system skills are only deployed when the launch's finalized environment actually carries
KANDEV_CLI— the same signal the Office scheduler path always sets. A routine-task agent simply doesn't receive skills it can't execute. User-authored skills (is_system = false) are unaffected.Who hits this: Any workspace running heavy Office routines; today every one of those agents hits a guaranteed failure on its first office-skill invocation.
Scope: standalone — narrow, single-purpose fix, no sibling PRs.
Not here: making routine tasks
IsFromOffice(rejected — that flag gates ~20 other behaviours, including a hard launch refusal intask_operations.go); injectingKANDEV_CLIalone (rejected — other required Office env vars, e.g.KANDEV_API_KEY, would still be missing, trading a clear absence for an auth failure instead).Important Changes
skill.SkillgainsIsSystem bool, threaded from config (office/skills/runtime_adapter.go) through to the deploy request.skill.Request.OfficeRuntime bool— set from whetherKANDEV_CLIis present in the finalized launch env (skill_deploy.go, at the point the env is already resolved) — gatesIsSystemskills out of the manifest (skill/manifest.go) when false.Known trade-off: the gate keys on
KANDEV_CLIbeing present in the finalized launch env. An install withagentctlBinaryPathunset (office/service/env_builder.go:33) never setsKANDEV_CLI, even for a genuine Office launch, so it would also stop receiving Office system skills. That's a mis-configured install, and a silently-absent skill is preferable to one guaranteed to fail — but it's a real trade-off, called out here rather than left implicit.Known gap (Kubernetes executor): on the k8s executor this gate is blunted by a pre-existing bug in
executor_kubernetes_files.go— it has nocleanKandevSkillsequivalent, so system skills already materialized on a retained PVC survive a manifest that now omits them. That bug is pre-existing, outside this diff, and tracked separately.Validation
go build -tags fts5 ./...— build OK;go vet ./...— OK.go test -tags fts5 -count=1 ./internal/agent/runtime/lifecycle/skill/... ./internal/office/skills/... ./internal/office/configloader/...— allok.golangci-lint run ./...(make -C apps/backend lint) —0 issues.make -C apps/backend test(full suite) — fails on the same pre-existing, environment-dependent packages as a scratch worktree built from this PR's merge base (temp-dir path length limits, unavailable NATS/turn-table test doubles, etc.); none of the failing packages exercise this diff. Two packages that failed only under full-suite contention (internal/common/subproc,internal/github) pass cleanly run in isolation.make typecheck,make lint,make lint-format,cd apps/web && pnpm run i18n:ratchet— all clean. (lint-harnessfails locally on a pre-existing Python 3.9-vs-3.10 syntax issue in.github/scripts/lint-harness-files.py, unrelated to and unchanged by this diff.)apps/webfiles changed, so Playwright e2e was not required for this change.Possible Improvements
Low risk. Coverage gap noted in review: no test currently spans the Office scheduler → lifecycle boundary, or exercises a prior-system-skills → gated transition end-to-end; this PR's tests cover the gate itself at the deploy-request/manifest level.
Checklist
apps/web/), I have added or updated Playwright e2e tests inapps/web/e2e/and verified them withmake test-e2e.docs/public/**and updated them or noted why no docs change is needed. (Backend-internal skill deployment behavior; no public docs reference this gate.)