Commit Graph

16 Commits

Author SHA1 Message Date
kshitijk4poor
54a867a6e9 fix(skill_manage): batch success rows carry the linter findings and org-sharing note
Since operations[] became the only call shape (#97295) the batch's success
path rebuilt each row as {name, action, file_path, success} and dropped every
advisory key the per-op handler attached — lint_warnings/lint_hint from the
create linter and the org_sharing note. The model never saw a finding.
Carry those keys onto the row, mirroring what the failure path already does
for teaching payloads.
2026-09-22 12:52:49 +05:30
teknium1
0bd85d314c fix(skills): keep absorbed_into in the delete shape; decide every patch shape miss pre-effect
The per-action schema made the delete branch `additionalProperties: false`
with no `absorbed_into`, so a schema-validating or grammar-constrained
backend could no longer emit the curator's consolidation delete and
`_curator_consolidation_delete_guard` fail-closed every consolidation.
Advertise `absorbed_into` on the delete branch and cover the batch path
that forwards it to the guard.

Also fold the two remaining patch shape checks (missing new_string,
content mixed with old_string/new_string) into `_op_shape_error`, so a
batch rejects them before applying any sibling instead of creating op[0]
and rolling it back. Drop the stale `edit` vocabulary from skills.md:440
and the curator prompt, and move the schema-diet test helpers above the
`__main__` guard.
2026-09-17 08:47:01 -07:00
teknium1
75a0649a18 fix(skills): skill_manage advertises one shape per action; a misfiled text slot fails before any op applies
The operations[] item schema was one flat object with four coexisting text
slots (content / new_string / file_content / file_path). A 27B local model
that had just used write_file's file_content kept emitting it on create and
patch ops; the call validated against the advertised schema, the handler
failed on "content is required", and the whole batch rolled back — eight
identical retries until the tool-loop guardrail tripped (#112677).

- items is now an anyOf of self-contained per-action op objects (create,
  patch targeted, patch full-rewrite, write_file, remove_file, delete), each
  with additionalProperties: false. The wire shape of a correct call is
  unchanged (still a flat op with name/action/...), so transcripts, staging
  and replay are untouched; grammar-constrained backends can no longer emit
  another action's slot, and schema-validating providers reject it up front.
  Nested (non-top-level) anyOf survives every sanitizer (schema_sanitizer,
  Gemini legacy translator). Cost: parameters JSON 1352 -> 2414 bytes.
- _validate_batch_ops runs the per-op argument-shape check (_op_shape_error)
  before any op is applied, so a misfiled op[1] no longer applies op[0] and
  then rolls the batch back; the error carries the same misplaced-key hint.
- The hint is attached to argument-shape misses only: a patch whose real
  problem is an unmatched old_string is no longer told to "move that text to
  'content' (full rewrite)", the escape the patch error itself warns against.
- Shape tables (_REQUIRED_ARGS, text-slot maps, _misplaced_text_hint,
  _op_shape_error) move out of the facade into skill_manager_batch.py, the
  op-validation sibling; _patch_skill shares the old_string guidance text.
- Docs: skills.md Actions table states the one-slot-per-action contract.
2026-09-17 08:47:01 -07:00
teknium1
273986f88f fix(skills): route the skill_manage lock through the existing skill_usage lock helper
Slim follow-up to the cherry-picked #111585 (@KoNit-K):

- tools/skill_usage.py: generalize the usage ledger's `_usage_file_lock()` into
  `skill_file_lock(lock_path)` — same fcntl/msvcrt idiom, now thread-re-entrant
  via a per-thread held set (flock is not re-entrant across separate fds; a
  ContextVar would leak "held" into copy_context() timer threads).
- tools/skill_manager_tool.py: drop the third fcntl/msvcrt copy, hashlib and the
  ContextVar; the per-skill lock is `<skills>/.locks/<skill-dir-name>.lock`
  (readable, outside the skill dir so delete/recreate cannot unlink it under a
  waiting writer). Batch locks sort by lock PATH, not name, so two batches
  naming the same skills in different forms cannot deadlock.
- tools/skill_manager_batch.py: plain `with` around snapshot -> commit/rollback
  instead of manual __enter__/__exit__ bookkeeping.
- tests: trimmed to two invariants — the two-writer lost-update test on
  SKILL.md (from #111585) and a re-entrancy/exclusivity test on the helper.
  Dropped: the edit/write_file/remove_file parametrization (same dispatcher
  path as patch) and the category-dir cleanup test (lock files never lived in
  category dirs here).
2026-09-15 19:03:33 -07:00
KoNit-K
0b8b000ce9 fix(skills): serialize skill mutations 2026-09-15 19:03:33 -07:00
Teknium
e83816a4d1 review-fix(comments): restore lost #NNNN rationale comments across non-test source (mechanical sweep, condensed, code unchanged)
For each issue anchor present in BASE 63279301bc non-test .py and absent on HEAD, the BASE comment/docstring block was re-attached at the HEAD location of the code it explained (matched by the distinctive code line / enclosing def). Sentences already covered by an existing HEAD comment were deduped; the issue number always survives. Insert-only: no code lines changed.
2026-09-03 09:44:26 -07:00
Teknium
e6048ff6aa refactor(tools): skill_manage/ledger — shared identifier check, chainable result decorators, ledger read/filter fold 2026-09-03 01:30:26 -07:00
Teknium
370c6a2456 refactor(tools): skill_manage — dispatch default for unknown action, simpler ledger read, squeeze body blanks 2026-09-03 01:26:03 -07:00
Teknium
4ba77a6c13 refactor(tools): skill_manage — shared missing-arg predicates, compact category validation and comments 2026-09-03 01:23:09 -07:00
Teknium
eeb86e4b6f refactor(tools): skill_manage — fold gate/flat-op plumbing, shared root/pin/org helpers in guards, ledger tarball lookup inline; compact docstrings 2026-09-03 01:17:57 -07:00
Teknium
d9d25c89ba refactor(tools): unify skill_manage guard/ledger boilerplate; compact batch + ledger helpers 2026-09-03 01:06:07 -07:00
Teknium
ce992963a2 refactor(tools): skill ledger/batch — inline one-use helpers, drop section banners 2026-09-02 23:10:14 -07:00
Teknium
a861976519 refactor(tools): skill guards/tool — is_relative_to collapses, string reflow 2026-09-02 23:03:24 -07:00
Teknium
b187d4e547 refactor(tools): skill linter checks as generators, skill_manage arg-shape table, suppress() collapses 2026-09-02 22:43:10 -07:00
Teknium
71dadfeb99 refactor(tools): trim skill_manage stack — dead linter CLI/format helpers, merged path resolvers, compact layout 2026-09-02 22:23:24 -07:00
Teknium
7c1ec19d4a refactor(tools/skills): split skills_hub into per-source modules; extract skill_manager guards/batch, skills_tool dedup/plugin/setup; compact skill_usage, skills_guard 2026-09-02 14:45:15 -07:00