feat(activity): add before_ts paging knob to /activity route
The wheel-side chat_history MCP tool advertises a `before_ts` parameter for backward paging through long histories, and the docs describe it as the canonical pagination knob — but the server silently ignored it until now. Without this fix, an agent passing before_ts to chat_history would always get the most-recent N rows and pagination would be broken end-to-end. Add `before_ts` query param parsed as RFC3339 at the trust boundary and translated into a `created_at < $X` clause on the existing builder. Mirrors the strict-inequality shape since_id uses for forward paging (`created_at > cursorTime`) so paging across both directions has consistent semantics. Tests: 3 new branches (positive filter, composition with peer_id into the canonical chat_history paging shape, RFC3339 rejection across 4 malformed inputs including URL-encoded SQL injection). Mutation-verified pre-commit; existing 9 activity tests still pass. Reported by self-review on PR #2474. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
645d687b0a
commit
15e1ea36de
@ -60,6 +60,7 @@ func (h *ActivityHandler) List(c *gin.Context) {
|
||||
limitStr := c.DefaultQuery("limit", "100")
|
||||
sinceSecsStr := c.Query("since_secs")
|
||||
sinceID := c.Query("since_id")
|
||||
beforeTSStr := c.Query("before_ts") // optional RFC3339 — return rows strictly older than this timestamp
|
||||
|
||||
// Validate peer_id as a UUID at the trust boundary so a malformed
|
||||
// caller (the agent or a downstream MCP tool) can't smuggle SQL
|
||||
@ -75,6 +76,25 @@ func (h *ActivityHandler) List(c *gin.Context) {
|
||||
}
|
||||
}
|
||||
|
||||
// Parse before_ts as the wall-clock paging knob for the wheel-side
|
||||
// `chat_history` MCP tool. The agent passes the oldest `created_at`
|
||||
// from a previous response to walk backward through long histories.
|
||||
// Validated as RFC3339 at the trust boundary so a typoed value
|
||||
// surfaces as a clean 400 instead of being silently ignored.
|
||||
var beforeTS time.Time
|
||||
usingBeforeTS := false
|
||||
if beforeTSStr != "" {
|
||||
t, err := time.Parse(time.RFC3339, beforeTSStr)
|
||||
if err != nil {
|
||||
c.JSON(http.StatusBadRequest, gin.H{
|
||||
"error": "before_ts must be an RFC3339 timestamp (e.g. 2026-05-01T00:00:00Z)",
|
||||
})
|
||||
return
|
||||
}
|
||||
beforeTS = t
|
||||
usingBeforeTS = true
|
||||
}
|
||||
|
||||
limit := 100
|
||||
if n, err := strconv.Atoi(limitStr); err == nil && n > 0 {
|
||||
limit = n
|
||||
@ -167,6 +187,14 @@ func (h *ActivityHandler) List(c *gin.Context) {
|
||||
args = append(args, peerID)
|
||||
argIdx++
|
||||
}
|
||||
if usingBeforeTS {
|
||||
// Strictly older — never replay a row with the exact same
|
||||
// timestamp, mirrors the `created_at > cursorTime` shape
|
||||
// `since_id` uses for forward paging.
|
||||
query += fmt.Sprintf(" AND created_at < $%d", argIdx)
|
||||
args = append(args, beforeTS)
|
||||
argIdx++
|
||||
}
|
||||
if sinceSecs > 0 {
|
||||
// Use a parameterized interval so the value is bound, not
|
||||
// interpolated into the SQL string. `make_interval(secs => $N)`
|
||||
|
||||
@ -280,6 +280,110 @@ func TestActivityList_PeerIDRejectsNonUUID(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// ---------- before_ts paging knob ----------
|
||||
//
|
||||
// before_ts is the wall-clock paging companion to peer_id — the agent
|
||||
// walks backward through long histories by passing the oldest
|
||||
// `created_at` from the previous response. Validated as RFC3339 at the
|
||||
// trust boundary; mirrors the strict-inequality shape since_id uses
|
||||
// for forward paging.
|
||||
|
||||
func TestActivityList_BeforeTSFilter(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
broadcaster := newTestBroadcaster()
|
||||
handler := NewActivityHandler(broadcaster)
|
||||
|
||||
cutoff, _ := time.Parse(time.RFC3339, "2026-05-01T00:00:00Z")
|
||||
mock.ExpectQuery(
|
||||
`SELECT .+ FROM activity_logs WHERE workspace_id = .+ AND created_at < .+`,
|
||||
).
|
||||
WithArgs("ws-1", cutoff, 100).
|
||||
WillReturnRows(sqlmock.NewRows([]string{
|
||||
"id", "workspace_id", "activity_type", "source_id", "target_id",
|
||||
"method", "summary", "request_body", "response_body",
|
||||
"tool_trace", "duration_ms", "status", "error_detail", "created_at",
|
||||
}))
|
||||
|
||||
gin.SetMode(gin.TestMode)
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: "ws-1"}}
|
||||
c.Request = httptest.NewRequest(
|
||||
"GET", "/workspaces/ws-1/activity?before_ts=2026-05-01T00%3A00%3A00Z", nil,
|
||||
)
|
||||
handler.List(c)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Fatalf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestActivityList_BeforeTSComposesWithPeerID(t *testing.T) {
|
||||
// peer_id + before_ts: the canonical wheel-side chat_history paging
|
||||
// shape. Pin both args + arg order so a future builder refactor
|
||||
// can't silently drop one filter or reorder placeholders.
|
||||
mock := setupTestDB(t)
|
||||
broadcaster := newTestBroadcaster()
|
||||
handler := NewActivityHandler(broadcaster)
|
||||
|
||||
cutoff, _ := time.Parse(time.RFC3339, "2026-05-01T00:00:00Z")
|
||||
mock.ExpectQuery(
|
||||
`SELECT .+ FROM activity_logs WHERE workspace_id = .+ AND \(source_id = .+ OR target_id = .+\) AND created_at < .+`,
|
||||
).
|
||||
WithArgs("ws-1", testPeerUUID, cutoff, 100).
|
||||
WillReturnRows(sqlmock.NewRows([]string{
|
||||
"id", "workspace_id", "activity_type", "source_id", "target_id",
|
||||
"method", "summary", "request_body", "response_body",
|
||||
"tool_trace", "duration_ms", "status", "error_detail", "created_at",
|
||||
}))
|
||||
|
||||
gin.SetMode(gin.TestMode)
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: "ws-1"}}
|
||||
c.Request = httptest.NewRequest(
|
||||
"GET",
|
||||
"/workspaces/ws-1/activity?peer_id="+testPeerUUID+"&before_ts=2026-05-01T00%3A00%3A00Z",
|
||||
nil,
|
||||
)
|
||||
handler.List(c)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Fatalf("unmet expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestActivityList_BeforeTSRejectsInvalidFormat(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
broadcaster := newTestBroadcaster()
|
||||
handler := NewActivityHandler(broadcaster)
|
||||
|
||||
for _, bad := range []string{
|
||||
"yesterday",
|
||||
"2026-05-01", // missing time component
|
||||
"2026-05-01%2000%3A00%3A00", // URL-encoded space instead of T
|
||||
"%27%20OR%201%3D1%20--", // URL-encoded SQL injection
|
||||
} {
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: "ws-1"}}
|
||||
c.Request = httptest.NewRequest(
|
||||
"GET", "/workspaces/ws-1/activity?before_ts="+bad, nil,
|
||||
)
|
||||
handler.List(c)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Errorf("before_ts=%q: expected 400, got %d (%s)", bad, w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ---------- Activity type allowlist (#125: memory_write added) ----------
|
||||
|
||||
func TestActivityReport_AcceptsMemoryWriteType(t *testing.T) {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user