Skip to content

feat(server): expose skill policy and knowledge review over HTTP - #106

Merged
tournierjc merged 1 commit into
mainfrom
feat/server-skill-review
Sep 30, 2026
Merged

tournierjc merged 1 commit into
mainfrom
feat/server-skill-review

Conversation

@tournierjc

@tournierjc tournierjc commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Change

#99 is merged. This PR adds the session-server HTTP surface so Android and other HTTP clients can drive that skill workflow:

  • POST /sessions/:id/skill-policy — mandatory / optional / forbidden dispositions and closed catalog
  • GET /skills/proposals — list Skill/Memory drafts awaiting review. Query: status, kind, sessionId, limit (default 50, clamped to the store's 200 cap)
  • POST /skills/proposals/:id/review — approve / reject (optional editedDraft, reviewNote). Reviewing an unknown proposal answers 404 and an already-reviewed one 409, so a client can tell a lost retry from a broken server

Also wires those methods onto TuiServerRuntime, documents them in docs/harness-integration.md and docs/tui-capabilities.md, and extends the server-http unit tests.

Supersedes #104: its head branch was renamed cursor/skill-http-after-99-9cd5 → feat/server-skill-review to match AGENTS.md, and GitHub closes a pull request whose head branch is renamed (docs; only base branches are repointed), so the old PR could not follow the rename. Same single commit ec67eed, same body — the review thread, the review follow-up comment and the gate results stay on #104, which itself replaced draft #102 (its branch still carried pre-merge #99 history).

Review fixes carried in this commit, each with a test: the proposals route is pinned to its two path shapes, limit is clamped to 200, and the knowledge-proposal domain errors map to 404/409 instead of a bare 500.

Validation

  • Checks run and results: pnpm verify at ec67eed — 15 gates passed (linux aarch64, Node 26.5.1, pnpm 9.12.0); skipped test:windows and test:sandbox as not applicable on this platform, and test:release-package as it needs an npm release archive. packages/tui/test/unit/server-http.test.ts (24 tests) is declared in test/vitest-suites.json under capability.
  • Negative checks: the two new boundary assertions fail against the pre-fix http.ts (500 instead of 404/409; limit=100000 forwarded unclamped), so they pin the fix rather than the fixture.
  • Performance: basic (HTTP route/capability wiring only; no new per-turn prompt assembly beyond feat: skill management with prompt lookup, idle drafts, and human review #99). No perf:full label — nothing here touches the paths the performance rules cover.
  • NOT RUN, platform limitations and live-service boundaries: no live-provider or live-service acceptance, no interactive TUI path, and this box is the only platform the gates ran on. GitHub CI is the platform record.

Publication and contribution checks

  • I have permission to contribute these changes under the existing licenses applicable to the changed files/packages; imported material and its provenance are identified and existing notices are preserved.
  • No credentials, account data, real user content, internal source history or private review material is included.
  • Added/removed source files were reviewed before regenerating release/public-source.json; new tests are declared in test/vitest-suites.json where applicable. (No source file added or removed: the change edits five existing paths, so the inventory is unchanged and check:source/check:tsconfig are unaffected.)
  • Shared English/Chinese documentation and capability/verification records are updated where applicable: the HTTP routes and their response shapes are in docs/harness-integration.md, and docs/tui-capabilities.md now points its capability record at the server surface. No localized product string changed, so no copy.zh-Hans entry applies. Mock/offline results are not described as live-service acceptance.

Maintainer handoff

Publication scope or license changes (if any): none.

Shared-source port: not needed.

Add the session-server routes for the Skill-management surface introduced in
#99, so a client other than the TUI can read and act on it:

- `POST /sessions/<id>/skill-policy` sets one Session's Skill dispositions.
- `GET /skills/proposals` lists Skill/Memory drafts awaiting human review.
- `POST /skills/proposals/<id>/review` approves or rejects one draft.

Every route is capability-optional: a runtime that does not expose the method
answers 404 with the same wording the existing write surface uses.

Review fixes on top of the first revision:

- Clamp `limit` to the draft store's own 200 cap, and pin the proposals route
  to its two path shapes so a stray `/skills/<other>` keeps falling through to
  the plain 404 tail whatever the runtime exposes.
- Translate the knowledge-proposal domain errors at the boundary: a proposal
  that is gone answers 404 and an already-reviewed one 409, instead of the bare
  500 a client cannot tell from a server fault.
- Make the skill-policy fake merge across calls and record what the runtime
  receives, so the suite pins forwarding (kind/limit, `null`-clearing) instead
  of echoing the request back; declare the third route in the minimal-runtime
  404 case.
- Document the review response shape and the limit cap in
  docs/harness-integration.md, and point docs/tui-capabilities.md at the HTTP
  surface.

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: hermes-agent <hermes-agent@users.noreply.github.com>
@tournierjc tournierjc added enhancement New feature or request cli labels Sep 30, 2026
@tournierjc
tournierjc merged commit b9b9c61 into main Sep 30, 2026
30 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant