Skip to content

feat(skills): add path-triggered rules#3994

Merged
VascoSch92 merged 14 commits into
mainfrom
3984-investigation-discussion-of-claude-rules
Jul 9, 2026
Merged

feat(skills): add path-triggered rules#3994
VascoSch92 merged 14 commits into
mainfrom
3984-investigation-discussion-of-claude-rules

Conversation

@VascoSch92

@VascoSch92 VascoSch92 commented Jul 5, 2026

Copy link
Copy Markdown
Member

HUMAN:

Add path-triggered skills, which behave like claude code rules.
You can fin a ready to run event in .pr


AGENT:

Why

Claude Code "Rules" (issue #3984) inject guidance deterministically: when a
file matching a glob is touched, the rule is guaranteed to load — no reliance on
the model choosing a skill, and no always-on frontmatter cost. This adds that to
the SDK by extending the existing Skill machinery rather than building a parallel
Rules subsystem.

Summary

  • New PathTrigger — a skill with paths: glob frontmatter. Its content is
    injected when the agent reads/edits/creates a matching file (gitignore-style
    ** globs, matched against the workspace-relative POSIX path). Fires on
    create too, beating Claude Code's Write/create gap.
  • Zero baseline cost — path rules are excluded from <available_skills> and
    are not model-invocable (disable_model_invocation is forced), so they never
    sit in always-on context.
  • Reuses existing machinery — a rule is just a skill, so loading, precedence,
    and serialization are unchanged (no new directory or concept). Injection rides
    on a new ObservationEvent.extended_content, deduped once per conversation via
    ConversationState.activated_path_rules.

REST API contract changes

Compared with base OpenAPI 61b7db85f6ea for public /api/** paths.

--- base public OpenAPI
+++ head public OpenAPI
@@ -1730,0 +1731 @@
+schema ObservationEvent property extended_content optional schema=type="array" items=TextContent
@@ -1752,0 +1754,3 @@
+schema PathTrigger property paths required schema=type="array" items=type="string"
+schema PathTrigger property type optional schema=type="string" const="path" default="path"
+schema PathTrigger type="object"
@@ -1968 +1972 @@
-schema Skill-Input property trigger optional schema=anyOf=[oneOf=[KeywordTrigger,TaskTrigger],type="null"]
+schema Skill-Input property trigger optional schema=anyOf=[oneOf=[KeywordTrigger,TaskTrigger,PathTrigger],type="null"]
@@ -1984 +1988 @@
-schema Skill-Output property trigger optional schema=anyOf=[oneOf=[KeywordTrigger,TaskTrigger],type="null"]
+schema Skill-Output property trigger optional schema=anyOf=[oneOf=[KeywordTrigger,TaskTrigger,PathTrigger],type="null"]

REST API contract changes

Compared with base OpenAPI 61b7db85f6ea for public /api/** paths.

--- base public OpenAPI
+++ head public OpenAPI
@@ -1730,0 +1731 @@
+schema ObservationEvent property extended_content optional schema=type="array" items=TextContent
@@ -1752,0 +1754,3 @@
+schema PathTrigger property paths required schema=type="array" items=type="string"
+schema PathTrigger property type optional schema=type="string" const="path" default="path"
+schema PathTrigger type="object"
@@ -1968 +1972 @@
-schema Skill-Input property trigger optional schema=anyOf=[oneOf=[KeywordTrigger,TaskTrigger],type="null"]
+schema Skill-Input property trigger optional schema=anyOf=[oneOf=[KeywordTrigger,TaskTrigger,PathTrigger],type="null"]
@@ -1984 +1988 @@
-schema Skill-Output property trigger optional schema=anyOf=[oneOf=[KeywordTrigger,TaskTrigger],type="null"]
+schema Skill-Output property trigger optional schema=anyOf=[oneOf=[KeywordTrigger,TaskTrigger,PathTrigger],type="null"]

REST API contract changes

Compared with base OpenAPI ebdb4566aca5 for public /api/** paths.

--- base public OpenAPI
+++ head public OpenAPI
@@ -1818,0 +1819 @@
+schema ObservationEvent property extended_content optional schema=type="array" items=TextContent
@@ -1840,0 +1842,3 @@
+schema PathTrigger property paths required schema=type="array" items=type="string"
+schema PathTrigger property type optional schema=type="string" const="path" default="path"
+schema PathTrigger type="object"
@@ -2056 +2060 @@
-schema Skill-Input property trigger optional schema=anyOf=[oneOf=[KeywordTrigger,TaskTrigger],type="null"]
+schema Skill-Input property trigger optional schema=anyOf=[oneOf=[KeywordTrigger,TaskTrigger,PathTrigger],type="null"]
@@ -2072 +2076 @@
-schema Skill-Output property trigger optional schema=anyOf=[oneOf=[KeywordTrigger,TaskTrigger],type="null"]
+schema Skill-Output property trigger optional schema=anyOf=[oneOf=[KeywordTrigger,TaskTrigger,PathTrigger],type="null"]

Issue Number

#3984

How to Test

End-to-end — real file_editor tool + real Conversation.run(), scripted
TestLLM (no API key). Committed at .pr/demo_path_rules.py:

uv run python .pr/demo_path_rules.py

Output:

====================================================================
rule loaded          : trigger=PathTrigger paths=['src/api/**/*.ts']
advertised in catalog: False  (should be False)
invocable by model   : False  (should be False)
activated_path_rules : ['api-rule']
LLM sees the rule after touching src/api/users.ts: True
====================================================================

The agent creates src/api/users.ts through the real file-editor tool; the rule
body (API RULE: validate all request inputs with zod.) is folded into the tool
observation the LLM reads on its next step, while the rule stays absent from the
catalog and non-invocable.

Tests

uv run pytest tests/sdk/skills/test_path_trigger.py \
  tests/sdk/conversation/test_path_rules_injection.py \
  tests/tools/test_path_rules_file_editor.py
# 61 passed

Regression (unchanged behavior): the full tests/sdk/{skills,context,event,tool}
and tests/sdk/conversation suites pass. pre-commit (ruff, ruff-format,
pyright, import-dependency rules) is clean on every changed file.

Video/Screenshots

CLI feature — text demo output shown above (no GUI).

Type

  • Bug fix
  • Feature
  • Refactor
  • Breaking change
  • Docs / chore

Notes

  • A skill is either path-triggered or model-invocable, not both: if a file
    declares both paths: and triggers:, paths: wins. This is deliberate (keeps
    rules deterministic and out of the catalog); easy to make them combinable if
    that's preferred.
  • ACP conversations do not get injection (ACP servers own tool execution, so file
    edits don't flow through the SDK observation seam) — a known boundary, not a
    regression.
  • Follow-ups intentionally out of scope: "always" rules (empty paths:
    REPO_CONTEXT) and packaging rules inside Plugins.

Agent Server images for this PR

GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server

Variants & Base Images

Variant Architectures Base Image Docs / Tags
java amd64, arm64 eclipse-temurin:17-jdk Link
python amd64, arm64 nikolaik/python-nodejs:python3.13-nodejs22-slim Link
golang amd64, arm64 golang:1.21-bookworm Link

Pull (multi-arch manifest)

# Each variant is a multi-arch manifest supporting both amd64 and arm64
docker pull ghcr.io/openhands/agent-server:39997a3-python

Run

docker run -it --rm \
  -p 8000:8000 \
  --name agent-server-39997a3-python \
  ghcr.io/openhands/agent-server:39997a3-python

All tags pushed for this build

ghcr.io/openhands/agent-server:39997a3-golang-amd64
ghcr.io/openhands/agent-server:39997a3ca6b09005c36cb7bb7f17964574a9aab3-golang-amd64
ghcr.io/openhands/agent-server:3984-investigation-discussion-of-claude-rules-golang-amd64
ghcr.io/openhands/agent-server:39997a3-golang_tag_1.21-bookworm-amd64
ghcr.io/openhands/agent-server:39997a3-golang-arm64
ghcr.io/openhands/agent-server:39997a3ca6b09005c36cb7bb7f17964574a9aab3-golang-arm64
ghcr.io/openhands/agent-server:3984-investigation-discussion-of-claude-rules-golang-arm64
ghcr.io/openhands/agent-server:39997a3-golang_tag_1.21-bookworm-arm64
ghcr.io/openhands/agent-server:39997a3-java-amd64
ghcr.io/openhands/agent-server:39997a3ca6b09005c36cb7bb7f17964574a9aab3-java-amd64
ghcr.io/openhands/agent-server:3984-investigation-discussion-of-claude-rules-java-amd64
ghcr.io/openhands/agent-server:39997a3-eclipse-temurin_tag_17-jdk-amd64
ghcr.io/openhands/agent-server:39997a3-java-arm64
ghcr.io/openhands/agent-server:39997a3ca6b09005c36cb7bb7f17964574a9aab3-java-arm64
ghcr.io/openhands/agent-server:3984-investigation-discussion-of-claude-rules-java-arm64
ghcr.io/openhands/agent-server:39997a3-eclipse-temurin_tag_17-jdk-arm64
ghcr.io/openhands/agent-server:39997a3-python-amd64
ghcr.io/openhands/agent-server:39997a3ca6b09005c36cb7bb7f17964574a9aab3-python-amd64
ghcr.io/openhands/agent-server:3984-investigation-discussion-of-claude-rules-python-amd64
ghcr.io/openhands/agent-server:39997a3-nikolaik_s_python-nodejs_tag_python3.13-nodejs22-slim-amd64
ghcr.io/openhands/agent-server:39997a3-python-arm64
ghcr.io/openhands/agent-server:39997a3ca6b09005c36cb7bb7f17964574a9aab3-python-arm64
ghcr.io/openhands/agent-server:3984-investigation-discussion-of-claude-rules-python-arm64
ghcr.io/openhands/agent-server:39997a3-nikolaik_s_python-nodejs_tag_python3.13-nodejs22-slim-arm64
ghcr.io/openhands/agent-server:39997a3-golang
ghcr.io/openhands/agent-server:39997a3ca6b09005c36cb7bb7f17964574a9aab3-golang
ghcr.io/openhands/agent-server:3984-investigation-discussion-of-claude-rules-golang
ghcr.io/openhands/agent-server:39997a3-golang_tag_1.21-bookworm
ghcr.io/openhands/agent-server:39997a3-java
ghcr.io/openhands/agent-server:39997a3ca6b09005c36cb7bb7f17964574a9aab3-java
ghcr.io/openhands/agent-server:3984-investigation-discussion-of-claude-rules-java
ghcr.io/openhands/agent-server:39997a3-eclipse-temurin_tag_17-jdk
ghcr.io/openhands/agent-server:39997a3-python
ghcr.io/openhands/agent-server:39997a3ca6b09005c36cb7bb7f17964574a9aab3-python
ghcr.io/openhands/agent-server:3984-investigation-discussion-of-claude-rules-python
ghcr.io/openhands/agent-server:39997a3-nikolaik_s_python-nodejs_tag_python3.13-nodejs22-slim

About Multi-Architecture Support

  • Each variant tag (e.g., 39997a3-python) is a multi-arch manifest supporting both amd64 and arm64
  • Docker automatically pulls the correct architecture for your platform
  • Individual architecture tags (e.g., 39997a3-python-amd64) are also available if needed

A path rule is a markdown skill with `paths:` glob frontmatter whose
content is injected deterministically when the agent touches a matching
file (read/edit/create) — no model decision. Rules stay out of the
<available_skills> catalog and are not model-invocable, so they add no
always-on context. Implemented as a new trigger on the existing Skill, so
loading, precedence, and serialization are reused (no new directory or
concept).

- skills: PathTrigger type, `paths:` parsing, gitignore-style `**` glob
  matcher, Skill.match_path_trigger; PathTrigger forces
  disable_model_invocation so catalog / invoke_skill / tool-attach exclude it
- context: AgentContext.get_tool_use_suffix renders matched rules
- events + conversation: ObservationEvent.extended_content carries the rule
  into the tool message; a callback seam reads the touched path and dedups
  via ConversationState.activated_path_rules

Related: #3984
@VascoSch92 VascoSch92 linked an issue Jul 5, 2026 that may be closed by this pull request
@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

📁 PR Artifacts Notice

This PR contains a .pr/ directory with PR-specific documents. This directory will be automatically removed when the PR is approved.

For fork PRs: Manual removal is required before merging.

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Python API breakage checks — ✅ PASSED

Result:PASSED

Action log

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

REST API breakage checks (OpenAPI) — ✅ PASSED

Result:PASSED

Action log

Comment thread openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py Outdated
Comment thread openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py Outdated
Comment thread openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py
Comment thread tests/sdk/conversation/test_path_rules_injection.py Outdated
Comment thread tests/sdk/skills/test_path_trigger.py Outdated
Comment thread tests/tools/test_path_rules_file_editor.py Outdated
@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Coverage

Coverage Report •
FileStmtsMissCoverMissing
openhands-sdk/openhands/sdk/context
   agent_context.py179597%437–438, 508, 531, 537
openhands-sdk/openhands/sdk/conversation
   state.py271697%355–356, 365–367, 543
openhands-sdk/openhands/sdk/conversation/impl
   local_conversation.py10199490%133, 584–585, 618, 635, 942, 966–967, 972, 987, 989, 1071, 1087, 1135, 1161, 1192, 1213, 1217–1223, 1282, 1302–1304, 1353, 1372–1374, 1696–1697, 1712, 1800–1801, 1804, 1937, 1940–1941, 1966, 1999–2000, 2003, 2009, 2090, 2097, 2100, 2103, 2107–2108, 2112–2113, 2116, 2123, 2148, 2152, 2155, 2174, 2226, 2229, 2268, 2275–2276, 2284, 2288–2290, 2297, 2334–2340, 2343, 2346, 2353, 2416, 2421, 2531, 2533, 2537–2538, 2549–2550, 2575, 2775, 2779, 2849, 2856–2857
openhands-sdk/openhands/sdk/event/llm_convertible
   observation.py69691%128–129, 134, 167–168, 173
openhands-sdk/openhands/sdk/skills
   skill.py5363194%153–154, 316, 319–320, 329, 580, 649, 730–733, 930–931, 1003–1004, 1063–1064, 1177–1178, 1264, 1292, 1315, 1322–1323, 1373–1374, 1380–1381, 1387–1388
   utils.py2241493%146, 164, 216, 249–250, 253, 270–271, 326, 432, 452, 473, 614–615
TOTAL356541015571% 

VascoSch92 and others added 8 commits July 6, 2026 07:20
Replace the manual index-scanning in `_compile_path_glob` with a single
tokenizer regex plus a `Final`-typed operator table; behavior is unchanged
(verified against the full glob test matrix). Also trims two docstrings.
… imports

- Add tests for two previously-uncovered defensive branches in the injection
  seam: an agent with no agent_context, and an observation whose action is
  not correlated in the event log.
- Drop `from __future__ import annotations` from the path-rule test files;
  their annotations are runtime-resolvable and pyright stays clean without it.
_touched_rule_path used PurePosixPath, which treats a Windows drive path
(e.g. D:\...) as relative — so the workspace-relative normalization was
skipped and path rules never matched on Windows. Use the native PurePath
flavour and emit as_posix() for glob matching. Fixes the windows-tests
failures on #3994.
@VascoSch92 VascoSch92 marked this pull request as ready for review July 6, 2026 08:24

@all-hands-bot all-hands-bot 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.

✅ QA Report: PASS

Verified path-triggered skill rules end-to-end with a real Conversation.run() and real file_editor creates; the PR delivers deterministic matching-file injection with zero baseline rule context.

Does this PR achieve its stated goal?

Yes. The stated goal is to add path-triggered skills that behave like deterministic rules: not advertised/invocable up front, then injected when a matching file is touched. On main, the same paths: skill was loaded as trigger_type: NoneType, appeared in the pre-run system suffix, and never produced injected tool-observation content. On this PR, the same real file-editor workflow loaded PathTrigger, kept the rule out of the initial system suffix/catalog, set disable_model_invocation: true, injected the rule into the first matching file observation, deduped the second matching touch, and did not inject for README.md.

Phase Result
Environment Setup make build completed and installed the uv workspace environment.
CI Status 🟡 Most completed checks are green; several agent-server image/build and QA checks were still in progress when sampled.
Functional Verification ✅ Real SDK conversation + file editor execution verified matching, dedupe, nonmatching, and baseline-cost behavior.
Functional Verification

Test 1: Matching path rule fires once and stays out of baseline context

Step 1 — Establish baseline without the fix:
Ran git switch main && OPENHANDS_SUPPRESS_BANNER=1 uv run python /tmp/qa_path_rules_probe.py with a temporary workspace containing .openhands/skills/api-rule.md using paths: ["src/api/**/*.ts"], then used a real SDK Conversation.run() and real file_editor creates for src/api/users.ts and src/api/groups.ts.

Relevant output:

{
  "trigger_type": "NoneType",
  "pre_run_system_suffix_contains_rule": true,
  "activated_path_rules": null,
  "extended_by_observation": [[], []],
  "llm_tool_messages_containing_rule": 0,
  "files_created": [true, true]
}

This shows the old behavior: the paths: frontmatter did not create a path trigger, the rule text was present before any file touch, and no tool-observation injection happened.

Step 2 — Apply the PR's changes:
Checked out 3984-investigation-discussion-of-claude-rules at 106774c6d292fe4a580038037a15fab6e152dbeb.

Step 3 — Re-run with the fix in place:
Ran OPENHANDS_SUPPRESS_BANNER=1 uv run python /tmp/qa_path_rules_probe.py with the same workspace shape and real file-editor operations.

Relevant output:

{
  "trigger_type": "PathTrigger",
  "disable_model_invocation": true,
  "advertised_in_available_skills": false,
  "pre_run_system_suffix_contains_rule": false,
  "activated_path_rules": ["api-rule"],
  "extended_by_observation": [["<EXTRA_INFO>...API RULE: validate all request inputs with zod....</EXTRA_INFO>"], []],
  "llm_tool_messages_containing_rule": 1,
  "files_created": [true, true]
}

This confirms the new behavior works end-to-end: the matching file create succeeds, the rule is injected into the LLM-visible tool message exactly once, and a second matching create is deduped in the same conversation.

Test 2: Nonmatching file does not trigger the rule

Step 1 — Establish baseline without the fix:
On main, the same temporary skill with paths: ["src/api/**/*.ts"] was not a path trigger and was present in baseline context even when the agent created README.md.

Relevant output:

{
  "target_rels": ["README.md"],
  "trigger_type": "NoneType",
  "pre_run_system_suffix_contains_rule": true,
  "extended_by_observation": [[]],
  "files_created": [true]
}

Step 2 — Re-run on the PR branch:
Used the same real SDK conversation and real file_editor create for README.md.

Relevant output:

{
  "target_rels": ["README.md"],
  "trigger_type": "PathTrigger",
  "pre_run_system_suffix_contains_rule": false,
  "activated_path_rules": [],
  "extended_by_observation": [[]],
  "llm_tool_messages_containing_rule": 0,
  "files_created": [true]
}

This confirms nonmatching file touches remain clean: the file is created, but the path rule is not injected or marked active.

Issues Found

None.

This review was created by an AI agent (OpenHands) on behalf of the user.

@VascoSch92 VascoSch92 changed the title feat(skills): add path-triggered rules (PathTrigger) feat(skills): add path-triggered rules Jul 6, 2026

@all-hands-bot all-hands-bot 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.

✅ QA Report: PASS

Verified path-triggered skills end-to-end with real Conversation.run() + real file_editor; the PR delivers deterministic rule injection for matching file touches and keeps rules out of model invocation paths.

Does this PR achieve its stated goal?

Yes. The PR set out to add Claude-rules-like path-triggered skills that load only when a matching file is touched, including create, without appearing in <available_skills> or being model-invocable. On main, the same paths: skill produced no path trigger state and no injected observation content; on this PR, creating src/api/users.ts injected the rule into the tool observation, recorded activated_path_rules: ["api-rule"], and kept advertised_in_catalog: false / disable_model_invocation: true. A nonmatching README.ts create still created the file but did not inject the rule.

Phase Result
Environment Setup make build completed successfully; no tests, linters, or pre-commit hooks were run manually.
CI Status ✅ GitHub reports 34 successful checks, 4 skipped, 0 failing, 0 pending.
Functional Verification ✅ Real SDK conversations with real file-editor operations matched the PR’s claimed behavior.
Functional Verification

Test 1: Baseline vs PR path-triggered rule injection

Step 1 — Establish baseline on main without the feature:
Ran git checkout main && OPENHANDS_SUPPRESS_BANNER=1 uv run python /tmp/path_rules_qa_probe.py using a temporary SDK script that creates .openhands/skills/api-rule.md with paths: ["src/api/**/*.ts"], runs a real Conversation.run(), and uses the real file_editor tool to create both src/api/users.ts and README.ts.

Relevant output:

{
  "matching_create": {
    "activated_path_rules": null,
    "created_file_exists": true,
    "disable_model_invocation": false,
    "extended_content": [],
    "llm_sees_rule_after_touch": false,
    "tool_message_contains_rule": false,
    "trigger_type": "NoneType"
  },
  "nonmatching_create": {
    "activated_path_rules": null,
    "created_file_exists": true,
    "extended_content": [],
    "llm_sees_rule_after_touch": false,
    "trigger_type": "NoneType"
  }
}

This shows the pre-PR SDK can load the skill file and create files through the tool, but it does not treat paths: as a path trigger and does not inject any rule into the tool observation.

Step 2 — Apply the PR changes:
Checked out 3984-investigation-discussion-of-claude-rules at 106774c6d292fe4a580038037a15fab6e152dbeb.

Step 3 — Re-run the same real SDK scenario with the PR:
Ran OPENHANDS_SUPPRESS_BANNER=1 uv run python /tmp/path_rules_qa_probe.py.

Relevant output:

{
  "matching_create": {
    "activated_path_rules": ["api-rule"],
    "advertised_in_catalog": false,
    "created_file_exists": true,
    "disable_model_invocation": true,
    "extended_content": ["<EXTRA_INFO>...API RULE: validate all request inputs with zod...</EXTRA_INFO>"],
    "llm_sees_rule_after_touch": true,
    "tool_message_contains_rule": true,
    "trigger_type": "PathTrigger"
  },
  "nonmatching_create": {
    "activated_path_rules": [],
    "advertised_in_catalog": false,
    "created_file_exists": true,
    "extended_content": [],
    "llm_sees_rule_after_touch": false,
    "trigger_type": "PathTrigger"
  }
}

This confirms the new behavior works end-to-end: a matching create operation injects the rule into the observation that becomes the next LLM tool message, while a nonmatching create does not.

Test 2: PR-provided end-to-end demo

Ran OPENHANDS_SUPPRESS_BANNER=1 uv run python .pr/demo_path_rules.py.

Relevant output:

Prompt Extension based on Agent Context:
<EXTRA_INFO>
The following rule applies because a file you touched matches "src/api/**/*.ts". Follow it when working with matching files.
Rule location: /tmp/tmp6ka9stl_/.openhands/skills/api-rule.md

API RULE: validate all request inputs with zod.
</EXTRA_INFO>

rule loaded          : trigger=PathTrigger paths=['src/api/**/*.ts']
advertised in catalog: False  (should be False)
invocable by model   : False  (should be False)
activated_path_rules : ['api-rule']
LLM sees the rule after touching src/api/users.ts: True

This independently confirms the user-facing demo path succeeds with the real file-editor tool and scripted TestLLM.

Issues Found

None.

This review was created by an AI agent (OpenHands) on behalf of the user.

@jpshackelford jpshackelford 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.

Path-Triggered Skills Testing Summary

🔬 How I Tested

Test 1: Plugin Integration Demo

  • Created a plugin with 2 path-triggered skills
  • Loaded the plugin using Plugin.load()
  • Executed file operations that matched the path patterns
  • Tool: TestLLM (scripted responses, no API calls)

Test 2: Keyword Deception Test

  • Created 3 rules with DISTINCT themes (animals, colors, numbers)
  • Used prompts with MISLEADING keywords (mention animals, but create frontend file)
  • Verified only path-matching rule fires (not keyword-matching)
  • Tool: TestLLM (full control over which files are touched)

Test 3: Real LLM Verification

  • Used generic prompts with NO theme keywords
  • Let LLM decide how to create files
  • Verified correct rules fire based on file paths
  • Tool: Real LLM via LiteLLM proxy (gpt-4o-mini)

✅ Results

Plugin Integration - PASS

✓ Created plugin with 2 path-triggered skills
✓ Loaded plugin successfully
✓ Both skills activated when matching files touched
✓ Rules injected into LLM context via extended_content
✓ Deduplication worked correctly

Activated path rules: ['typescript-rules', 'test-rules']

Keyword Deception Test - ALL PASS

Test 1: Prompt with ANIMAL keywords → created frontend/app.js
        Result: COLORS rule fired (not animals!) ✅

Test 2: Prompt with COLOR keywords → created backend/api.py  
        Result: ANIMALS rule fired (not colors!) ✅

Test 3: No keywords → created src/utils.py
        Result: NO rules fired ✅

Test 4: Numbers keywords → created data/schema.sql
        Result: NUMBERS rule fired ✅

PROVEN: Keywords in prompts have ZERO effect - only file paths trigger rules!

Real LLM Test - 4/5 PASS

Test 1: Generic prompt → LLM created backend/api.py
        Result: ANIMALS rule fired ✅

Test 2: Generic prompt → LLM created frontend/app.js
        Result: COLORS rule fired ✅

Test 3: Generic prompt → LLM created data/schema.sql
        Result: NUMBERS rule fired ✅

Test 4: Generic prompt → LLM created src/utils.py
        Result: NO rules fired ✅

Test 5: Nested path backend/nested/deep/file.py
        Result: Not fired (already activated - deduplication) ℹ️

🎓 Key Findings

  1. ✅ Plugin Support: Path-triggered skills work with plugins out of the box - no additional code needed

  2. ✅ Hook Compatibility: Hooks and path-triggered skills are complementary - they work together without conflicts

  3. ✅ Path-Based Triggering: Rules fire based on glob pattern matching on file paths, NOT semantic analysis or keyword detection

  4. ✅ Deterministic: Same path always triggers the same rule(s)

  5. ✅ Zero Baseline Cost: Rules excluded from <available_skills> catalog until triggered

  6. ✅ Deduplication: Each rule fires once per conversation (prevents redundant injections)

🚀 Conclusion

Path-triggered skills work exactly as designed:

  • Triggers are 100% path-based (proven with keyword deception tests)
  • Works seamlessly with plugins and hooks
  • Tested with both scripted and real LLM
  • Deterministic, efficient, and reliable

Comment thread openhands-sdk/openhands/sdk/context/agent_context.py
Comment thread openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py
Comment thread openhands-sdk/openhands/sdk/event/llm_convertible/observation.py

Copy link
Copy Markdown
Member

Suggestion: Add log warning for dropped triggers

When a skill defines both paths: and triggers:, the paths: takes precedence and triggers: is silently ignored. This could be confusing for users creating skills who don't realize one of their fields is being ignored.

Minimal in-scope fix

Add a logger.warning() in _create_skill_from_metadata when this happens:

if paths:
    # Warn if both paths and triggers/inputs are defined
    if keywords or "inputs" in metadata_dict:
        ignored = []
        if keywords:
            ignored.append(f"'triggers': {keywords}")
        if "inputs" in metadata_dict:
            ignored.append("'inputs'")
        
        logger.warning(
            f"Skill '{agent_name}' has both 'paths:' and {', '.join(ignored)} defined. "
            f"Using 'paths:' (path-triggered rule). {', '.join(ignored)} will be ignored. "
            f"A skill can be either path-triggered OR model-invocable, not both."
        )

While this only appears in server logs (not the conversation UI), it at least makes the behavior discoverable for debugging.

Follow-up issue

For user-visible skill loading warnings in the conversation UI (similar to HookExecutionEvent), I've opened #4015 which describes the broader infrastructure needed. That would be out of scope for this PR but valuable for future work.


This comment was created by an AI agent (OpenHands) on behalf of the reviewer.

A skill declaring both `paths:` and `triggers:`/`inputs:` becomes a
path-triggered rule and silently drops the model-invocable fields. Emit a
warning so the dropped fields are discoverable instead of vanishing.
Nested instruction files (AGENTS.md, CLAUDE.md, .cursorrules, ...) in
subdirectories become PathTrigger rules scoped to their directory, injected only
when the agent touches a file underneath them; the root file keeps its always-on
behavior. Discovery uses `git ls-files` (tracked + untracked-not-ignored) with a
pruned filesystem-walk fallback when git is unavailable.
_touched_rule_path resolved the touched file to a workspace-relative path
with PurePath.relative_to, which does no symlink resolution. When the
action path and the workspace root differed only by a symlink (e.g. macOS
/tmp -> /private/tmp), relative_to raised and the path rule silently never
fired. Add a filesystem-resolution fallback (strict=False, so a not-yet-
created leaf is preserved) before concluding the file is outside the
workspace.
Comment thread openhands-sdk/openhands/sdk/skills/skill.py
@enyst

enyst commented Jul 8, 2026

Copy link
Copy Markdown
Member

@OpenHands Describe to me the behavior of this PR when the repository looks like this repository itself : that is, as you can see, it has nested AGENTS.md files, not only in root dir, but also in some subdirs. This PR attempts to load them sometimes: describe to me when it loads e.g. AGENTS.md from openhands-agent-server.

Respond directly as a comment on github here.

Then. Once you responded. Take a deep breath, and /codereview this PR. Post your feedback also on the PR.

@openhands-fleet-stable

Copy link
Copy Markdown

@enyst it looks like you haven't created an OpenHands account yet. Please sign up at OpenHands Cloud and try again.

@openhands-ai

openhands-ai Bot commented Jul 8, 2026

Copy link
Copy Markdown

I'm on it! enyst can track my progress at all-hands.dev

enyst commented Jul 8, 2026

Copy link
Copy Markdown
Member

@enyst Good question — this repo is actually a perfect stress test, because it has an AGENTS.md at the root and five nested ones. Here's exactly what this PR does with them (numbers below are from running the real loader against this checkout).

Two different code paths for root vs. nested

The root AGENTS.md is not a path rule. It's picked up by find_third_party_files() and loaded as an always-on repo skill (trigger=None), exactly as before this PR. It sits in permanent context regardless of what files you touch.

The nested ones are the new behavior. find_nested_third_party_files() walks the worktree (via git ls-files --cached --others --exclude-standard, i.e. tracked + non-ignored files), finds every AGENTS.md that is not at the top level, and _handle_nested_third_party() turns each into a PathTrigger "rule" scoped to its own directory. On this checkout that produces:

agents:openhands-agent-server                  -> ['openhands-agent-server/**']
agents:openhands-sdk/openhands/sdk             -> ['openhands-sdk/openhands/sdk/**']
agents:openhands-sdk/openhands/sdk/subagent    -> ['openhands-sdk/openhands/sdk/subagent/**']
agents:openhands-tools/openhands/tools         -> ['openhands-tools/openhands/tools/**']
agents:openhands-workspace/openhands/workspace -> ['openhands-workspace/openhands/workspace/**']

The rule name encodes the directory (agents:<rel_dir>) so the five files that all map to the agents third-party name stay distinct for dedup/activation tracking.

When does openhands-agent-server/AGENTS.md load?

That file becomes the rule agents:openhands-agent-server with glob openhands-agent-server/**. It is injected the first time the agent touches a file whose workspace-relative POSIX path matches that glob — read, edit, or create through the file-editor tool (any tool whose action carries a path). Verified against the real matcher:

Touched path Injected?
openhands-agent-server/openhands/agent_server/config.py ✅ yes
openhands-agent-server/pyproject.toml ✅ yes
openhands-agent-server/AGENTS.md (the file itself) ✅ yes
openhands-agent-server (the bare dir path) ❌ no — ** requires a segment after the slash
openhands-sdk/openhands/sdk/llm/llm.py ❌ no
openhands-agent-server-other/x.py ❌ no — not fooled by the shared prefix

Mechanically: when the tool ObservationEvent comes back, _maybe_inject_path_rules() correlates it to its ActionEvent, reads the action's path, relativizes it against the workspace root, and asks the agent context for matching PathTrigger skills. The matched rule's entire AGENTS.md body is folded into that observation's new extended_content, so the LLM sees it appended right after the tool result on its next step — wrapped in an <EXTRA_INFO> block explaining the rule fired because a touched file matched openhands-agent-server/**.

Three behaviors worth calling out for this repo specifically

  1. Once per conversation. Injection is deduped via state.activated_path_rules. The first touch under openhands-agent-server/ injects the whole file; every subsequent touch under that dir is silent. So it isn't re-paid on every edit.

  2. Nested overlap = multiple rules fire. The globs are prefix-scoped, so touching openhands-sdk/openhands/sdk/subagent/foo.py matches both openhands-sdk/openhands/sdk/** and openhands-sdk/openhands/sdk/subagent/** (I confirmed both match). Editing a subagent file therefore injects both the sdk AGENTS.md and the sdk/subagent AGENTS.md (each once). That's arguably the intended "parent + child guidance" behavior, but worth being deliberate about — the parent file is large.

  3. Never advertised, never model-invocable. These rules are forced to disable_model_invocation=True and excluded from <available_skills>, so they cost nothing until a matching file is actually touched. Injection only happens through the SDK observation seam, so ACP-backed conversations don't get it (a known boundary noted in the PR).

One caveat: matching is entirely on the workspace-relative path, and only fires when the touched file resolves inside the workspace root (absolute paths are relativized; symlinked roots are handled via a resolve() fallback). If the workspace working dir were pointed at a subdirectory rather than the repo root, the relative paths — and thus which nested files load — would shift accordingly.


This comment was generated by an AI agent (OpenHands) on behalf of @enyst.

@enyst enyst 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.

Code Review — feat(skills): add path-triggered rules

🟡 Acceptable — the core design is genuinely good taste: a "rule" is just a Skill with a PathTrigger, so loading, precedence, serialization, and dedup all ride on machinery that already exists instead of a parallel Rules subsystem. The injection seam (ObservationEvent.extended_content, mutated on a frozen model via model_copy, appended in to_llm_message) is clean and mirrors the existing message-suffix path. Tests exercise real code paths (real LocalConversation, real events, real file-editor tool — no "assert the mock was called"), and they pass locally (76 passed). Nice work.

I'm not approving, for one policy reason and a couple of small nits — details below.

[IMPROVEMENT OPPORTUNITIES]

  • [local_conversation.py:490] getattr guard on a declared field. agent_context is a declared field on AgentBase (agent/base.py:209), so getattr(self.agent, "agent_context", None) can just be self.agent.agent_context. The repo conventions explicitly discourage getattr/hasattr guards in favor of typed access. I see this mirrors the pre-existing pattern at line 418, so it's not net-new — but since you're adding a fresh call site, worth doing it the typed way (and maybe cleaning up 418 while you're here).

  • [local_conversation.py:525] getattr(action_event.action, "path", None) is an implicit contract. This is a deliberate, pragmatic decoupling from the tools package (you call it out in the code comment, 👍). The flip side is that any action with a path field now drives rule matching. Today that's fine (file-editor's path is a workspace file path), but it silently assumes "a field named path == a workspace file path." If a future tool adds a path field meaning something else (a URL path, a registry key), it would spuriously trigger rules. Not blocking, but the assumption deserves to be explicit — either a short note in the docstring or a follow-up when a second path-bearing tool appears.

  • [agent_context.py:536] if not isinstance(skill, Skill) on a list[Skill] is a redundant defensive guard (mirrors the existing one at 489). Minor; skip if you're matching the sibling method's style deliberately.

[BEHAVIORAL NOTE — nested/overlapping rules]

Because rule globs are directory-prefix-scoped (<dir>/**), overlapping nested rules all fire. In a repo shaped like this one, touching openhands-sdk/openhands/sdk/subagent/foo.py matches both openhands-sdk/openhands/sdk/** and openhands-sdk/openhands/sdk/subagent/**, so it injects both the parent sdk/AGENTS.md and the child sdk/subagent/AGENTS.md (each once). That's a reasonable "parent + child guidance" semantic, but it's an intentional-looking design decision with real context-cost implications (the parent AGENTS.md here is large), and I don't see a test pinning that behavior. Consider adding one so the "inherit up the tree" contract is explicit and can't silently regress into "only the deepest match fires" (or vice-versa). See my walkthrough comment for the full breakdown of how this repo's six AGENTS.md files get partitioned.

[TESTING]

Real tests, no mock-theater — good. Two gaps worth a line each, neither blocking:

  • No test for the nested-overlap double-injection described above.
  • No test that a non-file tool carrying an unrelated path field does / doesn't trigger (locks in the implicit contract from the getattr note).

Backward-compat on the new ObservationEvent.extended_content field is fine: it has a default_factory, and extra="forbid" only rejects unknown keys, so old persisted observations load unchanged.

[RISK ASSESSMENT]

  • [Overall PR] ⚠️ Risk Assessment: 🟡 MEDIUM

This PR deterministically injects new content into the LLM's context on file-touch — that is a change to what the model sees on every matching step, which squarely lands in the eval/benchmark-risk category for this repo. The mechanism itself is well-gated (deduped once per conversation, excluded from <available_skills>, forced non-invocable, backward-compatible event field), so the engineering risk is low-to-medium. The behavioral risk is the open question: injecting large nested AGENTS.md bodies mid-conversation can shift agent behavior and benchmark scores in ways unit tests can't catch.

Per this repo's review policy, I can't approve an agent-behavior/prompt-injection change without eval evidence. I don't see an openhands-eval-monitor.vercel.app link in the description or comments.

Recommendation: Do not auto-merge. A human maintainer should run a lightweight eval and confirm no regression before this ships. Once there's an eval-monitor link + maintainer sign-off, this is close to mergeable modulo the two small nits above.

VERDICT: 🟣 Commenting, deferring to human maintainer — solid, well-tested implementation; blocked only on eval validation (repo policy) plus a couple of optional cleanups.

KEY INSIGHT: Modeling a "rule" as a path-triggered skill is the right call — it eliminates a whole parallel subsystem — but the moment rules inject full nested AGENTS.md bodies into live context, this becomes a benchmark-sensitive change that needs eval proof, not just green unit tests.


Improve this review? If any feedback here seems incorrect or irrelevant to this repository, add/edit .agents/skills/custom-codereview-guide.md on your branch with the /codereview trigger and the missing context, then re-request review. See the customization docs.

Was this review helpful? React with 👍 or 👎.


This review was generated by an AI agent (OpenHands) on behalf of @enyst.

@OpenHands OpenHands deleted a comment from openhands-ai Bot Jul 8, 2026
Comment thread openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py Outdated
@enyst

enyst commented Jul 8, 2026

Copy link
Copy Markdown
Member

@OpenHands Read the above comment (the last comment) and see this :

local_conversation.py:525] getattr(action_event.action, "path", None) is an implicit contract

and all the improvement opportunity discussed about path. Open a new issue where you document it, and propose a few alternative options (not for this PR, for a follow up clean up). Note that importing tools is not an option; but surely we can think of a better code design pattern?

Do not modify this PR.

@openhands-ai

openhands-ai Bot commented Jul 8, 2026

Copy link
Copy Markdown

I'm on it! enyst can track my progress at all-hands.dev

@enyst enyst 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.

Looks good to me, thank you for addressing the alternative AGENTS.md issue nicely!

(Not for this PR, maybe then we can also clean up the root file.)

enyst commented Jul 8, 2026

Copy link
Copy Markdown
Member

@enyst Done — opened #4038 to document the implicit getattr(action_event.action, "path", None) contract and propose follow-up alternatives (not for this PR).

It captures:

  • Why it's fragile: the SDK depends on the string literal "path" matching a field on tool actions it can't import/type-check, and it already silently misses tools — Gemini file tools use file_path and apply_patch uses patch text / move_path, so path rules don't fire there today.
  • The constraint: no importing openhands-tools from openhands-sdk.
  • Four alternatives: (A) a runtime_checkable Protocol (touched_paths()) tools opt into; (B) a field-level Annotated[..., FsPath()] semantic marker; (C) preferred — the producer reports affected_paths on the Observation (typed, multi-path, no correlation lookup); (D) a dependency-inverted extractor registry.

Recommendation leans to (C) with (A) as a lighter option. This PR is left unchanged.

This comment was created by an AI agent (OpenHands) on behalf of @enyst.

@OpenHands OpenHands deleted a comment from openhands-ai Bot Jul 8, 2026
@enyst

enyst commented Jul 8, 2026

Copy link
Copy Markdown
Member

cc @VascoSch92 just for visibility:

it already silently misses tools — Gemini file tools use file_path and (GPT-5.x) apply_patch uses patch text / move_path, so path rules don't fire there today.

- Replace getattr(self.agent, "agent_context", None) with direct typed
  access; agent_context is a declared field on AgentBase and AGENTS.md
  discourages getattr/hasattr guards over typed access.
- Add a test pinning that nested AGENTS.md rules with ancestor/descendant
  globs (pkg/** and pkg/sub/**) both inject when a file under the child
  directory is touched ("inherit up the tree").
@VascoSch92

Copy link
Copy Markdown
Member Author

cc @VascoSch92 just for visibility:

it already silently misses tools — Gemini file tools use file_path and (GPT-5.x) apply_patch uses patch text / move_path, so path rules don't fire there today.

Yes I know. I was just overfitting on the default tools that we are using. People using OH have access just to these tools. But we can have a follow-up for that.

…scussion-of-claude-rules

# Conflicts:
#	openhands-sdk/openhands/sdk/skills/skill.py
@VascoSch92 VascoSch92 enabled auto-merge (squash) July 9, 2026 09:46
@VascoSch92 VascoSch92 merged commit 7ce74f7 into main Jul 9, 2026
29 of 30 checks passed
@VascoSch92 VascoSch92 deleted the 3984-investigation-discussion-of-claude-rules branch July 9, 2026 09:47
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.

[Investigation] Discussion of Claude Rules

4 participants