Conversation
…tion - Add `skill-selector.ts` with `selectSkillsWithLLM()` — uses small model (Haiku 4.5 via `Provider.getSmallModel`) + `generateObject` to semantically select relevant skills based on user message and project fingerprint - Remove `partitionByFingerprint()` and `rescueByMessage()` from `tool/skill.ts` - Remove `tags` field from `Skill.Info` schema and all 51 SKILL.md files - Add 2 new dbt-labs skills: `migrating-dbt-core-to-fusion`, `migrating-dbt-project-across-platforms` - Rewrite tests with `SkillSelectorDeps` DI interface for model + generate mocking - Graceful fallback: returns all skills on timeout (3s), error, no model, or zero selection
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| cwd: string | ||
| } | ||
|
|
||
| let cached: Result | undefined |
There was a problem hiding this comment.
Bug: A race condition exists in the Fingerprint module's module-level cache, allowing concurrent sessions with different directories to overwrite and read each other's cached data.
Severity: MEDIUM
Suggested Fix
Refactor the Fingerprint cache to be session-aware or directory-aware instead of using a global singleton. This could be achieved by using a Map keyed by the directory cwd or by storing the fingerprint data within the session's context, similar to how other session-specific data is managed.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/opencode/src/altimate/fingerprint/index.ts#L15
Potential issue: The `Fingerprint` module uses a single module-level variable `cached`
to store fingerprint data. In a concurrent environment with multiple sessions operating
on different directories (`cwd`), a race condition can occur. If Session A detects a
fingerprint for `/project-a` and then yields, Session B can overwrite the cache with
data for `/project-b`. When Session A resumes and calls `Fingerprint.get()`, it will
incorrectly receive the fingerprint data from Session B. While the impact is currently
limited to incorrect skill selection, which has a graceful fallback, this represents a
state corruption issue across sessions.
Did we get this right? 👍 / 👎 to inform future reviews.
|
This pull request has been automatically closed because it was not updated to meet our contributing guidelines within the 2-hour window. Feel free to open a new pull request that follows our guidelines. |
1 participant
Summary
partitionByFingerprint+rescueByMessage) with a single Haiku LLM call that semantically selects relevant skills based on user message and project fingerprinttagsfield fromSkill.Infoschema and all 51 SKILL.md frontmatter files — no longer neededMessageContextside-channel andFingerprintdetection for environment-aware contextexperimental.dynamic_skillsconfig flag — disabled by defaultHow it works
selectSkillsWithLLM()inskill-selector.ts:Provider.getSmallModel()(Haiku 4.5 priority)generateObject()with zod schemaPromise.race— never blocks the main modelTest plan
bun test test/altimate/skill-filtering.test.ts— 20 tests covering all fallback paths, model verification, prompt contentbun test test/tool/skill.test.ts— existing tests passbunx tsc --noEmit— no type errorsdbt-modeling+sql-optimization, "deploy to k8s" → selectskubernetes-deploy, unrelated message → fallback to all skills