chore: address remaining copilot nits and clarify docs
This commit is contained in:
parent
db436aa5ca
commit
fed2093260
6 changed files with 31 additions and 8 deletions
|
|
@ -1012,6 +1012,7 @@ PicoClaw provides typed lifecycle hooks for observability, outbound filtering, a
|
||||||
- If hooks are not set, default behavior is unchanged.
|
- If hooks are not set, default behavior is unchanged.
|
||||||
|
|
||||||
See runnable examples: [docs/hooks-plugin-examples.md](docs/hooks-plugin-examples.md)
|
See runnable examples: [docs/hooks-plugin-examples.md](docs/hooks-plugin-examples.md)
|
||||||
|
|
||||||
Roadmap for plugin system evolution: [docs/design/plugin-system-roadmap.md](docs/design/plugin-system-roadmap.md)
|
Roadmap for plugin system evolution: [docs/design/plugin-system-roadmap.md](docs/design/plugin-system-roadmap.md)
|
||||||
|
|
||||||
<details>
|
<details>
|
||||||
|
|
|
||||||
|
|
@ -4,7 +4,7 @@ This document defines how PicoClaw evolves from hook-based extension points to a
|
||||||
|
|
||||||
## Current Status (Phase 0: Foundation)
|
## Current Status (Phase 0: Foundation)
|
||||||
|
|
||||||
Implemented in current hooks MR:
|
Implemented in current hooks PR:
|
||||||
|
|
||||||
- Typed lifecycle hooks (`pkg/hooks`)
|
- Typed lifecycle hooks (`pkg/hooks`)
|
||||||
- Priority-based handler ordering
|
- Priority-based handler ordering
|
||||||
|
|
@ -27,19 +27,20 @@ Compatibility:
|
||||||
|
|
||||||
## Phase Plan
|
## Phase Plan
|
||||||
|
|
||||||
## Phase 1: Static Plugin Contract (Compile-time)
|
## Phase 1: Static Plugin Contract (Compile-time) — Implemented
|
||||||
|
|
||||||
Goal: define a minimal public plugin contract for Go modules.
|
Goal: define a minimal public plugin contract for Go modules.
|
||||||
|
|
||||||
Proposed:
|
Implemented:
|
||||||
|
|
||||||
- Add `pkg/plugin` with a small interface:
|
- Add `pkg/plugin` with a small interface:
|
||||||
- `Name() string`
|
- `Name() string`
|
||||||
|
- `APIVersion() string`
|
||||||
- `Register(*hooks.HookRegistry) error`
|
- `Register(*hooks.HookRegistry) error`
|
||||||
- Register plugins at startup in code.
|
- Register plugins at startup in code.
|
||||||
- Add compatibility metadata (`PluginAPIVersion`) for forward checks.
|
- Add compatibility metadata (`plugin.APIVersion`) and registration-time checks.
|
||||||
|
|
||||||
Exit criteria:
|
Exit criteria (met):
|
||||||
|
|
||||||
- Example plugin module builds against the contract.
|
- Example plugin module builds against the contract.
|
||||||
- Startup validation logs loaded plugins and registration errors clearly.
|
- Startup validation logs loaded plugins and registration errors clearly.
|
||||||
|
|
@ -104,4 +105,4 @@ Until then, compile-time registration remains the recommended model.
|
||||||
|
|
||||||
## Maintainer Review Notes
|
## Maintainer Review Notes
|
||||||
|
|
||||||
The current hooks MR should be reviewed as Phase 0 only. It intentionally establishes extension points while avoiding high-risk runtime plugin mechanics.
|
The current hooks PR should be reviewed as Phase 0+1 only. It intentionally establishes extension points while avoiding high-risk runtime plugin mechanics.
|
||||||
|
|
|
||||||
|
|
@ -50,6 +50,10 @@ Inbound message
|
||||||
-> session_end
|
-> session_end
|
||||||
```
|
```
|
||||||
|
|
||||||
|
Note: the map above is shown as a single pass for readability. In practice, the
|
||||||
|
agent loop may iterate up to `MaxToolIterations`, and `llm_input`, `llm_output`,
|
||||||
|
`before_tool_call`, and `after_tool_call` can fire multiple times.
|
||||||
|
|
||||||
## Available Hooks
|
## Available Hooks
|
||||||
|
|
||||||
| Hook | Type | Typical use |
|
| Hook | Type | Typical use |
|
||||||
|
|
|
||||||
|
|
@ -913,7 +913,7 @@ func (al *AgentLoop) updateToolContexts(agent *AgentInstance, channel, chatID st
|
||||||
}
|
}
|
||||||
|
|
||||||
// maybeSummarize triggers summarization if the session history exceeds thresholds.
|
// maybeSummarize triggers summarization if the session history exceeds thresholds.
|
||||||
func (al *AgentLoop) maybeSummarize(ctx context.Context, agent *AgentInstance, sessionKey, channel, chatID string) {
|
func (al *AgentLoop) maybeSummarize(_ context.Context, agent *AgentInstance, sessionKey, channel, chatID string) {
|
||||||
newHistory := agent.Sessions.GetHistory(sessionKey)
|
newHistory := agent.Sessions.GetHistory(sessionKey)
|
||||||
tokenEstimate := al.estimateTokens(newHistory)
|
tokenEstimate := al.estimateTokens(newHistory)
|
||||||
threshold := agent.ContextWindow * 75 / 100
|
threshold := agent.ContextWindow * 75 / 100
|
||||||
|
|
@ -924,7 +924,7 @@ func (al *AgentLoop) maybeSummarize(ctx context.Context, agent *AgentInstance, s
|
||||||
go func() {
|
go func() {
|
||||||
defer al.summarizing.Delete(summarizeKey)
|
defer al.summarizing.Delete(summarizeKey)
|
||||||
if !constants.IsInternalChannel(channel) {
|
if !constants.IsInternalChannel(channel) {
|
||||||
al.sendOutbound(ctx, bus.OutboundMessage{
|
al.sendOutbound(context.Background(), bus.OutboundMessage{
|
||||||
Channel: channel,
|
Channel: channel,
|
||||||
ChatID: chatID,
|
ChatID: chatID,
|
||||||
Content: "Memory threshold reached. Optimizing conversation history...",
|
Content: "Memory threshold reached. Optimizing conversation history...",
|
||||||
|
|
|
||||||
|
|
@ -218,6 +218,8 @@ func clampArgNumber(args map[string]any, key string, max int) {
|
||||||
func toInt(v any) (int, bool) {
|
func toInt(v any) (int, bool) {
|
||||||
maxInt := int(^uint(0) >> 1)
|
maxInt := int(^uint(0) >> 1)
|
||||||
maxIntU64 := uint64(maxInt)
|
maxIntU64 := uint64(maxInt)
|
||||||
|
maxInt64 := int64(maxInt)
|
||||||
|
minInt64 := -maxInt64 - 1
|
||||||
|
|
||||||
switch n := v.(type) {
|
switch n := v.(type) {
|
||||||
case int:
|
case int:
|
||||||
|
|
@ -229,6 +231,9 @@ func toInt(v any) (int, bool) {
|
||||||
case int32:
|
case int32:
|
||||||
return int(n), true
|
return int(n), true
|
||||||
case int64:
|
case int64:
|
||||||
|
if n < minInt64 || n > maxInt64 {
|
||||||
|
return 0, false
|
||||||
|
}
|
||||||
return int(n), true
|
return int(n), true
|
||||||
case uint:
|
case uint:
|
||||||
if uint64(n) > maxIntU64 {
|
if uint64(n) > maxIntU64 {
|
||||||
|
|
@ -250,8 +255,10 @@ func toInt(v any) (int, bool) {
|
||||||
}
|
}
|
||||||
return int(n), true
|
return int(n), true
|
||||||
case float32:
|
case float32:
|
||||||
|
// Truncation is intentional for timeout normalization.
|
||||||
return int(n), true
|
return int(n), true
|
||||||
case float64:
|
case float64:
|
||||||
|
// Truncation is intentional for timeout normalization.
|
||||||
return int(n), true
|
return int(n), true
|
||||||
default:
|
default:
|
||||||
return 0, false
|
return 0, false
|
||||||
|
|
|
||||||
|
|
@ -2,6 +2,7 @@ package demoplugin
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"strconv"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
|
@ -171,3 +172,12 @@ func TestPolicyDemoPluginNoConfigNoEffect(t *testing.T) {
|
||||||
t.Fatalf("did not expect content rewrite, got %q", msgEvent.Content)
|
t.Fatalf("did not expect content rewrite, got %q", msgEvent.Content)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestToIntRejectsInt64OverflowOn32Bit(t *testing.T) {
|
||||||
|
if strconv.IntSize != 32 {
|
||||||
|
t.Skip("overflow scenario is specific to 32-bit int")
|
||||||
|
}
|
||||||
|
if _, ok := toInt(int64(1 << 40)); ok {
|
||||||
|
t.Fatal("expected overflow conversion to fail on 32-bit int")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue