Skip to content

fix(harness): preserve RuntimeContext during skill promotion - #2465

Open
RainYuY wants to merge 2 commits into
agentscope-ai:mainfrom
RainYuY:fix/skill-promotion-runtime-context
Open

fix(harness): preserve RuntimeContext during skill promotion#2465
RainYuY wants to merge 2 commits into
agentscope-ai:mainfrom
RainYuY:fix/skill-promotion-runtime-context

Conversation

@RainYuY

@RainYuY RainYuY commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

AgentScope-Java Version

2.0.1-SNAPSHOT

Description

Background

Skill promotion did not consistently preserve the caller's RuntimeContext.

For USER- or SESSION-scoped workspaces, the context determines which filesystem namespace contains the draft skill. Although SkillPromoter.promote(...) accepted a context, the following operations still used the default context or RuntimeContext.empty():

  • Draft skill lookup
  • Supporting resource discovery and reading
  • Moving the approved skill into the live directory

This could cause a user-scoped draft to appear missing or make promotion operate on the wrong namespace.

Changes

  • Added HarnessAgent.promoteSkill(name, reviewerId, RuntimeContext) for explicit context propagation.
  • Kept the existing overload backward-compatible by delegating with the agent's current context.
  • Added context-aware skill lookup to RuntimeContextSkillRepository.
  • Propagated the same effective context through:
    • Draft lookup
    • Resource loading and security scanning
    • Promotion gate review
    • Filesystem move
  • Added regression tests for USER-scoped skill isolation.

Expected Behavior

  • A user can promote a draft from their own namespace.
  • Another user cannot see or promote that draft.
  • Supporting files are scanned in the owning user's namespace.
  • Dangerous resources are rejected before invoking the promotion gate.

@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@oss-maintainer oss-maintainer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review: fix(harness): preserve RuntimeContext during skill promotion

Verdict: Approved ✅

Clean, well-scoped fix. The changes correctly propagate the caller's RuntimeContext through the entire skill promotion pipeline (draft lookup → resource loading → security scan → gate review → filesystem move).

Highlights

  • Backward-compatible overloadpromoteSkill(name, reviewerId) delegates to the new context-aware overload with getRuntimeContext(), preserving existing callers.
  • Default method on RuntimeContextSkillRepositorygetSkill(name, context) provides a sensible default implementation via getAllSkills(context), avoiding breaking changes for implementors.
  • SkillPromoter refactor — removes the scattered RuntimeContext.empty() / default-context usage and threads the effective context consistently.
  • Regression test — 111-line test covering USER-scoped isolation (draft visibility, cross-user rejection, resource scanning in owning namespace, dangerous resource rejection).

Minor observation

  • RuntimeContextSkillRepository.getSkill() does a linear scan. Fine for typical skill counts, but worth noting if repositories grow large.

LGTM.

@RainYuY

RainYuY commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

What is the intended semantic of skill promotion?

  1. Promote a draft within its existing USER/SESSION scope
  2. Publish an approved draft into the GLOBAL skill catalog
  3. Make the destination scope explicitly configurable

The current implementation follows option 1 because it preserves isolation and is consistent with the existing single-context filesystem API. I would appreciate maintainer feedback before finalizing this behavior. @chickenlj @jujn

@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants