chore(agent): drop the dead LOAD_SKILL_USE_ALLOWED_TOOLS constant - #3480
Conversation
Leftover from veryfront/veryfront-issue-inbox#406. The constant read "For multi-step or isolated work, call invoke_agent; otherwise keep working directly with the available tools" - copy written when a skill's allowed-tools narrowed a run. That enforcement was removed in #3464, and the string was reworded rather than deleted at the time. Nothing consumes it: no production caller, no test, and no other constant in delegation-policy.ts composes it. That distinguishes it from the neighbouring constants, which look unreferenced by name but are folded into reminders that ship - LOAD_SKILL_CONTINUATION_REMINDER, for one, is injected on the child-fork path by child-fork-execution-runner.ts. Removes the export from the veryfront/agent barrel and regenerates the API reference. Claude-Session: https://claude.ai/code/session_01Xo93b6StAu691YV9g8Fm53
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (2)
📝 WalkthroughWalkthroughThe change removes ChangesDelegation policy API cleanup
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a24488c640
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| LOAD_SKILL_ROOT_OWNERSHIP, | ||
| LOAD_SKILL_USE_ALLOWED_TOOLS, | ||
| NO_DELEGATION_NARRATION_UNLESS_ASKED, |
There was a problem hiding this comment.
Preserve the published load-skill export
When a consumer imports LOAD_SKILL_USE_ALLOWED_TOOLS from veryfront/agent, removing this entry breaks that consumer on upgrade: deno.json maps the public ./agent export to this index, and the symbol was included in the generated API reference before this change. The absence of internal references does not make a published export safe to remove, so retain it, optionally as deprecated, unless this is an explicitly breaking release.
AGENTS.md reference: AGENTS.md:L9-L10
Useful? React with 👍 / 👎.
|
Thanks both. CodeRabbit: no actionable comments. Codex — "Preserve the published load-skill export" (P2): declining, with reasonsThe finding is factually correct: 1. Keeping it is worse than breaking it. The constant's text is:
That is prompt copy describing a run whose tool set a skill had narrowed. #3464 removed that behaviour. A consumer importing this string today gets policy for a feature that no longer exists — and because it is prompt copy, the likely use is injecting it into agent instructions, which propagates stale guidance into their prompts silently. A compile error tells them to stop; a preserved constant does not. Its own history makes the point: when #3464 removed the enforcement I reworded this string ("allowed tools" → "available tools") instead of deleting it. Rewording bought a sentence that still had no subject. This PR finishes that job rather than repeating it. 2. Removing exports from this barrel is established practice here, including changes that are not mine:
Two of those were also in the generated API reference before removal. This PR removes one export — strictly less than #3473, which merged. 3. The removal is visible, not silent. I'll add it back behind a |
Leftover from veryfront/veryfront-issue-inbox#406.
LOAD_SKILL_USE_ALLOWED_TOOLSread:That sentence was written when a skill's
allowed-toolsnarrowed a run. #3464 removed the enforcement, and I reworded the string ("allowed tools" → "available tools") instead of deleting it. The rewording was the wrong call: with nothing narrowing the tool set, the sentence had no subject left.Why this one and not its neighbours
Several constants in
delegation-policy.tslook unreferenced if you grep for their names, because the code that ships them composes them inside the same file. This one is different:LOAD_SKILL_USE_ALLOWED_TOOLSLOAD_SKILL_CONTINUATION_REMINDERchild-fork-execution-runner.ts:213-217LOAD_SKILL_CONTINUATION_REMINDER,SLASH_COMMAND_ARTIFACT_REMINDER,FIRST_TURN_STARTER_INTENT_ROOT_OWNERSHIP_REMINDER,NO_DELEGATION_NARRATION_UNLESS_ASKED,SYNTHESIZE_DELEGATED_FINDINGS_IN_ROOT_VOICE, andROOT_OWNED_CHILD_RESULT_INSTRUCTIONall reach production throughaddLoadSkillContinuationReminder,addSlashCommandArtifactReminder,buildInvokeAgentFollowupInstruction, orwithRootOwnedChildResultHint. They stay.Only
LOAD_SKILL_USE_ALLOWED_TOOLShas no path to a model at all.Scope
src/agent/conversation/delegation-policy.tsveryfront/agentbarrel (src/agent/index.ts)docs/api-reference/veryfront/agent.mdThis is a public API removal, consistent with #3464 and #3475, which removed exports in the same area.
Testing
src/skill/,src/agent/,src/internal-agents/— 1195 passed, 2178 steps, 0 faileddeno task typecheck— 0 errorsdeno lint,lint:test-typecheck,docs:api-reference:check— all OK🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Refactor