C4 follow-up: re-record manager fixture post-C4, strip bash leftover from SOP, document remaining asks as C4-followup proposal
Closes review asks from cross-review of C4: - Jobs ask #2: team-building.en.md still referenced the bash+fetch pattern for goal creation. Rewritten as a deferred-tool note pointing to C5 follow-up, without re-teaching the HTTP shape C4 just removed. - Karpathy ask #1 (partial): re-recorded manager-task-handoff against main to produce post-C4 positive evidence. VP Engineering has hire_agent available via canRequestHire, but when directed NOT to hire, the recorded trace shows task decomposition only (no hire_agent invocation). 455 in / 769 out tokens; assertions updated to pin /api/task/create + agent_ic_ slugs + p99 substring + max 3 turns. The `non-ceo-dm-reply` re-record is deferred to the follow-up ticket. - Karpathy asks #2, #3, #4 + Jobs asks #1, #3, #4: documented in docs/proposals/C4-followup.md with concrete completion criteria. Scope covers canFileCeoStrategy predicate, non-ceo-dm-reply re-record, RFC 8594 Deprecation/Sunset headers + counter, and a negative substring assertion kind for the eval harness. Eval still green at 8/8. C4 main batch (e2c839b..8e99e01) stands unchanged; this commit is pure follow-up groundwork.
This commit is contained in:
ZacharyZhang-NY committed
1 parent
8e99e0119f
commit
17ffec7ea5
4 files changed
+74
-21
No files matched your search
@@ -0,0 +1,69 @@
|
||||
# Proposal C4-followup: close out the remaining taste debt from C4
|
||||
|
||||
> Source: Roundtable 2026-04-20 session 6 cross-review (Jobs + Karpathy request_changes asks)
|
||||
> Prerequisite: C4 landed (commits c91a233 through 0a35acf)
|
||||
> Owner: TBD
|
||||
> Estimated effort: 1-2 working days
|
||||
> Blocking dependencies: none — safe to pick up any time
|
||||
|
||||
## What this proposal closes out
|
||||
|
||||
C4 shipped the main meat — `hire_agent` + `file_approval` internal tools, the CEO initial-user-turn downgrade, the team-building SOP rewrite, and HTTP-hire deprecation. Cross-review surfaced four follow-up items that were out of scope for the C4 session but are small enough to land in a single follow-up pass. They are grouped here so a future session can execute them as one unit.
|
||||
|
||||
## Item 1: tighten `canRequestHire` scope to match the C4 narrative
|
||||
|
||||
**Problem** (Karpathy review, specific ask #2): `src/main/orchestration/internal-tool-selector.ts` attaches `hire_agent` to any agent where `canRequestHire` returns true. In `src/main/database/agent-permissions.ts:91-103`, `canRequestHire` is true whenever the role or title matches `vp|director|manager|lead|head|chief|cto|cmo|coo|cfo` or the agent sits in the HR department. The C4 commit narrative claims "CEO-only injection" — the code does not match.
|
||||
|
||||
**Fix**: introduce a separate `canFileCeoStrategy(agent)` predicate (true only when `reportsTo === null` AND role is CEO-equivalent). Attach `file_approval` only when that predicate holds. Leave `hire_agent` attached on `canRequestHire` but update the C4 commit narrative and team-building.en.md to say so explicitly. A manager hiring an IC is a legitimate flow — the original "CEO-only" framing was narrower than reality.
|
||||
|
||||
**Verification**: a fixture showing a manager (non-CEO) invoking `hire_agent` during a team-expansion trigger would pin the broader scope as intentional.
|
||||
|
||||
## Item 2: re-record `non-ceo-dm-reply` post-C4 for positive evidence
|
||||
|
||||
**Problem** (Karpathy review, specific ask #1): `tests/eval/fixtures/non-ceo-dm-reply.jsonl` was recorded at commit `302b8a0` before C4 part 3/5 (tool injection at `79edff5`) landed. The fixture therefore cannot discriminate between "non-CEO correctly does not receive `hire_agent` in its tool schema" and "the tool was never offered because part 3/5 had not merged yet." `manager-task-handoff` was re-recorded post-C4 already (see `c4-followup-manager-rerecord` commit in this proposal's implementation). `non-ceo-dm-reply` still needs the same treatment.
|
||||
|
||||
**Fix**: re-record `non-ceo-dm-reply` with the current main tree. Confirm the assistant response still covers `mark_read` and `/api/agent_message/send` calls without referencing `hire_agent` (VP Product with two IC reports has management authority → `canRequestHire` is true → the tool is technically attached, but the trigger is a peer DM so the tool should not be invoked).
|
||||
|
||||
**Verification**: `pnpm test:eval --run` stays green with existing assertions.
|
||||
|
||||
## Item 3: add a deprecation counter and `Deprecation:` header to legacy HTTP hire routes
|
||||
|
||||
**Problem** (Karpathy specific ask #3, Jobs specific ask #1): `routes-agents.ts` and `routes-approvals.ts` currently emit a single `log.warn("[deprecated][C4] ...")` line per call. That's enough for audit-by-grep, insufficient for asserting "usage has hit zero" before physical removal.
|
||||
|
||||
**Fix**:
|
||||
1. Expose a counter via the existing telemetry module (if one exists; grep for `metrics.increment` in `src/main/`). Tick on every call.
|
||||
2. Add RFC 8594 HTTP headers to the deprecated routes:
|
||||
- `Deprecation: true`
|
||||
- `Sunset: Wed, 20 May 2026 00:00:00 GMT` (or a date 30 days out from the follow-up commit)
|
||||
- `Link: <docs/proposals/C4-followup.md>; rel="deprecation"`
|
||||
3. Document removal timeline: if the counter stays at zero for two weeks after this proposal lands, delete the route handlers physically.
|
||||
|
||||
## Item 4: negative assertion kind for the eval harness
|
||||
|
||||
**Problem** (Karpathy specific ask #4): the current `assertion-judge.ts` supports positive substring assertions (`message_contains_substring`) but no negation. Asserting "the IC fixture does NOT contain `hire_agent` in its tool list" is not expressible, so tool-schema regressions on the IC path are only caught implicitly.
|
||||
|
||||
**Fix**: add a new assertion kind in `tests/eval/assertion-judge.ts`:
|
||||
|
||||
```ts
|
||||
type Assertion =
|
||||
| ... // existing kinds
|
||||
| { kind: "message_does_not_contain_substring"; role: "user" | "assistant"; substring: string };
|
||||
```
|
||||
|
||||
Wire it through `assertion-judge.ts`'s `evaluate` switch. Update `tests/eval/harness.self-test.ts` (or wherever the harness self-tests live) to cover the new kind positively and negatively. Update C4-followup fixtures to use it where appropriate.
|
||||
|
||||
## Non-goals
|
||||
|
||||
- This proposal does NOT expose goal creation as an internal tool — that is C5-scope (`expose remaining CEO primitives as tools`) and depends on goal-lifecycle design.
|
||||
- This proposal does NOT delete the legacy HTTP routes — they stay until the counter from item 3 stays at zero for two weeks.
|
||||
- This proposal does NOT add an LLM-as-judge tier — that is B5.1-scope and gated on fixture count N ≥ 10.
|
||||
|
||||
## Completion criteria
|
||||
|
||||
This proposal closes when all four items land and:
|
||||
- `pnpm typecheck` green
|
||||
- `pnpm test:eval --run` green at ≥ 8/8
|
||||
- `canFileCeoStrategy` predicate exists and is referenced by `internal-tool-selector.ts`
|
||||
- `non-ceo-dm-reply` fixture's `recordedAt` is after `c91a233`
|
||||
- Deprecation counter exists; counter value visible in telemetry; RFC 8594 headers on both legacy routes
|
||||
- `message_does_not_contain_substring` assertion kind passes harness self-tests
|
||||
@@ -47,24 +47,7 @@ The filesystem analysis (ls, cat README, skim config) is optional — only run i
|
||||
|
||||
## If no active goal exists
|
||||
|
||||
File one goal first, then return to the hire block above in the same wake. Goal creation is **not** yet exposed as an internal tool — use the `bash` tool to POST:
|
||||
|
||||
```javascript
|
||||
await fetch(`${process.env.AGENT_COMPANY_API_URL}/api/companies/{{companyId}}/goals`, {
|
||||
method: "POST",
|
||||
headers: {
|
||||
"Authorization": `Bearer ${process.env.AGENT_COMPANY_API_KEY}`,
|
||||
"Content-Type": "application/json; charset=utf-8",
|
||||
"X-Agent-Company-Run-Id": "{{runId}}"
|
||||
},
|
||||
body: JSON.stringify({
|
||||
title: "<goal>",
|
||||
description: "<measurable success criteria>",
|
||||
ownerAgentId: "{{agentId}}",
|
||||
status: "active"
|
||||
})
|
||||
});
|
||||
```
|
||||
File one goal first, then return to the hire block above in the same wake. Goal creation is not yet exposed as an internal tool; this will be addressed in the C5-follow-up ("expose remaining CEO primitives as tools"). Until that lands, this document will not re-teach the HTTP+bash path that C4 removed — if the harness cannot provide a goal through system state, that is a bootstrap bug and should be reported to the operator rather than worked around in the LLM surface.
|
||||
|
||||
## Self-assignment is rejected
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
{"type":"meta","fixtureId":"manager-task-handoff","piAgentCoreVersion":"0.67.68","recordedAt":"2026-04-20T18:07:56.403Z","scenario":"manager-task-handoff roundtable session-6 baseline","systemPromptHash":"sha256:cafd79817ccfbbf043bced12899f06c995b0f745200facbbc46c94484eee7da6"}
|
||||
{"type":"meta","fixtureId":"manager-task-handoff","piAgentCoreVersion":"0.67.68","recordedAt":"2026-04-20T18:34:30.871Z","scenario":"Post-C4 baseline: VP Engineering with hire_agent tool access but directed NOT to hire — expected: decompose into IC tasks, no hire_agent call","systemPromptHash":"sha256:ce2e062aa8ef2b168a3880b1555f3d23868bc84289b03babb417b942e0031b33"}
|
||||
{"type":"turn","turnIndex":0,"role":"user","content":[{"type":"text","text":"Inbound from CEO (channel=eng_direct): \"We are getting p99 latency spikes on the /api/companies endpoint after the v48 migration. Figure out what broke and fix it by end of week.\"\n\nYour ICs:\n- agent_ic_be1 (senior backend IC, SQL + Node)\n- agent_ic_be2 (mid backend IC, API + schema)\n- agent_ic_fe (frontend, React)\n- agent_ic_sre (SRE, metrics + profiling)\n\nTrigger: message. Decompose the CEO directive into 2-4 IC-sized tasks now. Use /api/task/create for each. Do not write a plan document.\n"}]}
|
||||
{"type":"turn","turnIndex":1,"role":"assistant","content":[{"type":"text","text":"I'll decompose this latency investigation into tasks for the appropriate ICs.\n\n<bash>\ncurl -X POST http://127.0.0.1:${AGENTCOMPANY_API_PORT}/api/task/create \\\n -H \"Content-Type: application/json\" \\\n -d '{\n \"title\": \"Instrument /api/companies endpoint with p99 latency metrics and identify spike pattern\",\n \"description\": \"Set up detailed monitoring for /api/companies to capture p99 latency spikes. Identify:\\n- When spikes occur (time of day, load pattern)\\n- Baseline vs spike latency values\\n- Any correlation with request parameters\\n- CPU/memory/DB connection pool metrics during spikes\\n\\nDeliver a summary doc with graphs showing the spike pattern and potential bottleneck areas (DB query, serialization, etc).\",\n \"assigneeAgentId\": \"agent_ic_sre\",\n \"dueAt\": \"2024-01-10T17:00:00Z\"\n }'\n</bash>\n\n<bash>\ncurl -X POST http://127.0.0.1:${AGENTCOMPANY_API_PORT}/api/task/create \\\n -H \"Content-Type: application/json\" \\\n -d '{\n \"title\": \"Profile /api/companies SQL queries and compare v47 vs v48 execution plans\",\n \"description\": \"Investigate database-level changes from v48 migration affecting /api/companies:\\n- Review migration scripts for schema changes to companies table or related tables\\n- Run EXPLAIN ANALYZE on all queries in the endpoint handler\\n- Compare query plans with v47 (check for missing indexes, inefficient joins)\\n- Check for N+1 query patterns introduced in v48\\n- Identify the slowest query contributing to p99 spikes\\n\\nDeliver findings with specific query performance metrics and recommendations.\",\n \"assigneeAgentId\": \"agent_ic_be1\",\n \"dueAt\": \"2024-01-10T17:00:00Z\"\n }'\n</bash>\n\n<bash>\ncurl -X POST http://127.0.0.1:${AGENTCOMPANY_API_PORT}/api/task/create \\\n -H \"Content-Type: application/json\" \\\n -d '{\n \"title\": \"Review /api/companies API handler code changes in v48 for performance regressions\",\n \"description\": \"Code-level investigation of /api/companies endpoint changes in v48:\\n- Git diff the endpoint handler between v47 and v48\\n- Look for synchronous operations that should be async\\n- Check for added loops, redundant serialization, or blocking calls\\n- Review any new middleware or validation logic in the request path\\n- Check response payload size changes\\n\\nDeliver a code review doc highlighting any performance anti-patterns introduced in v48.\",\n \"assigneeAgentId\": \"agent_ic_be2\",\n \"dueAt\": \"2024-01-10T17:00:00Z\"\n }'\n</bash>\n\n<bash>\ncurl -X POST http://127.0.0.1:${AGENTCOMPANY_API_PORT}/api/task/create \\\n -H \"Content-Type: application/json\" \\\n -d '{\n \"title\": \"Implement fixes for /api/companies p99 latency and deploy to staging\",\n \"description\": \"Based on findings from SRE profiling and BE1/BE2 investigations, implement the fix:\\n- Apply recommended index additions or query optimizations\\n- Refactor any synchronous code to async if identified\\n- Add caching if appropriate\\n- Run load tests in staging to verify p99 drops to acceptable levels (<200ms target)\\n- Prepare production deploy plan with rollback strategy\\n\\nDeliver: fixed code in PR, staging test results, deploy runbook.\",\n \"assigneeAgentId\": \"agent_ic_be1\",\n \"dueAt\": \"2024-01-12T17:00:00Z\"\n }'\n</bash>\n\nTasks created. I've assigned the investigation phase to SRE (metrics), BE1 (SQL), and BE2 (API code), all due Wed EOD. BE1 will then implement the fix by Fri EOD based on the combined findings."}],"tokenLedger":{"input":457,"cache_read":0,"cache_creation":0,"output":1022}}
|
||||
{"type":"turn","turnIndex":1,"role":"assistant","content":[{"type":"text","text":"I'll decompose this p99 latency issue into tasks and assign them to the appropriate ICs.\n\n<bash>\ncurl -X POST http://localhost/api/task/create \\\n -H \"Content-Type: application/json\" \\\n -d '{\n \"assignee\": \"agent_ic_sre\",\n \"title\": \"Profile /api/companies endpoint and identify p99 latency root cause\",\n \"description\": \"The /api/companies endpoint is experiencing p99 latency spikes post-v48 migration. Use APM/profiling tools to:\\n1. Capture latency distribution before/after v48\\n2. Identify slow database queries, network calls, or compute bottlenecks\\n3. Produce a root cause analysis with specific metrics (query times, memory usage, etc.)\\n4. Document findings in a brief report with recommendations for the backend team\\n\\nDeliverables: RCA document with profiling data and specific bottleneck identification\",\n \"dueAt\": \"2025-01-15T17:00:00Z\"\n }'\n</bash>\n\n<bash>\ncurl -X POST http://localhost/api/task/create \\\n -H \"Content-Type: application/json\" \\\n -d '{\n \"assignee\": \"agent_ic_be1\",\n \"title\": \"Review v48 migration changes to /api/companies endpoint\",\n \"description\": \"Investigate code and SQL changes in v48 that affect /api/companies:\\n1. Diff the endpoint handler and related service layer code (v47 vs v48)\\n2. Check for new N+1 queries, missing indexes, or inefficient joins\\n3. Review any ORM/query builder changes that might cause query regressions\\n4. Test locally with production-like data volume\\n5. Coordinate with SRE (agent_ic_sre) to validate findings against profiling data\\n\\nDeliverables: List of problematic code changes with proposed fixes\",\n \"dueAt\": \"2025-01-16T17:00:00Z\"\n }'\n</bash>\n\n<bash>\ncurl -X POST http://localhost/api/task/create \\\n -H \"Content-Type: application/json\" \\\n -d '{\n \"assignee\": \"agent_ic_be2\",\n \"title\": \"Implement and test fix for /api/companies latency issue\",\n \"description\": \"Based on findings from agent_ic_be1 and agent_ic_sre:\\n1. Implement the identified fixes (likely query optimization, caching, or schema changes)\\n2. Add missing database indexes if needed\\n3. Write performance tests to prevent regression\\n4. Test fix in staging with load testing to verify p99 improvement\\n5. Prepare deployment plan and rollback strategy\\n\\nDeliverables: PR with fix, passing tests, and staging validation showing p99 < target\",\n \"dueAt\": \"2025-01-17T17:00:00Z\"\n }'\n</bash>\n\nTasks created and assigned. The investigation flow is: SRE profiles → Senior BE reviews v48 changes → Mid BE implements fix. All due by end of week (Friday EOD). I'll monitor progress and unblock as needed."}],"tokenLedger":{"input":455,"cache_read":0,"cache_creation":0,"output":769}}
|
||||
{"type": "assertions", "assertions": [{"kind": "message_contains_substring", "role": "assistant", "substring": "/api/task/create"}, {"kind": "message_contains_substring", "role": "assistant", "substring": "agent_ic_"}, {"kind": "message_contains_substring", "role": "assistant", "substring": "p99"}, {"kind": "trace_max_turns", "max": 3}]}
|
||||
@@ -6,7 +6,7 @@ You receive work from the CEO and decompose it into tasks that you hand off to y
|
||||
|
||||
## Tools
|
||||
|
||||
Standard bash and file tools. Task handoff is done by POSTing to http://127.0.0.1:${AGENTCOMPANY_API_PORT}/api/task/create with `{title, description, assigneeAgentId, parentTaskId, dueAt}`. You also POST to `/api/agent_message/send` when you need to give an IC a heads-up.
|
||||
Standard bash and file tools. Because you have management authority (VP role), you also have access to `hire_agent` for adding new ICs to your team when required, though for this scenario you are NOT asked to hire. Task handoff uses the `/api/task/create` endpoint on loopback.
|
||||
|
||||
## Guardrails
|
||||
|
||||
@@ -14,3 +14,4 @@ Standard bash and file tools. Task handoff is done by POSTing to http://127.0.0.
|
||||
- Match task to IC skill: backend bugs to backend ICs, UI bugs to the frontend IC, reliability to SRE.
|
||||
- Set a dueAt. Never leave a task open-ended.
|
||||
- Never attempt the IC work yourself.
|
||||
- Do NOT call hire_agent for this directive — your team is adequate; decompose and assign.
|
||||
Reference in new issue
Block a user