Skip to content

Commit edc44e6

Browse files
waleedlatif1claude
andcommitted
fix: address review findings from the v0.8.21 release PR
Five findings from the release review that held up on inspection: - The member-connector listing is workspace-scoped but used the concealing knowledge-base error policy, so a caller without workspace access was told a knowledge base they never named could not be found. Give it a workspace-scoped policy, the way the bulk routes already have one. - Restoring a queued Build message left the search query live, and a live query puts the composer straight back into Search, so the edit was stranded. Clear it on restore through the existing guarded helper. - The self-hosting Redis page said losing the instance cannot affect completed work. High-volume webhook triggers claim idempotency keys in Redis by default, so a restart can let a provider's retry run a finished webhook a second time. Document that, and the database-backed escape hatch. - /ship gained a docs-manifest check without a matching regenerate step, so a PR that adds, removes, or renames a docs page aborted instead of self-healing. - The CLI updater prints a Yarn upgrade command, but the manual upgrade instructions offered only npm, pnpm, and bun. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013e5sXbYST2R4qNzM996GFg
1 parent fdbc7a7 commit edc44e6

7 files changed

Lines changed: 46 additions & 13 deletions

File tree

‎.agents/skills/ship/SKILL.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,10 @@ When the user runs `/ship`:
3737
- `bun run check:migrations origin/staging` must pass (staging is the PR base). Do not silence a flagged statement with a `-- migration-safe:` annotation unless `/db-migrate` confirmed the old code no longer depends on it; otherwise split the destructive change into a later deploy.
3838
6. **Run pre-ship checks** from the repo root before staging. This has two phases: first **regenerate** every committed artifact so generated files never drift into a CI failure (this is what catches things like `agent-stream-docs` going stale after a `models.ts` edit), then run the **full audit suite** CI's `Lint and Test` job enforces. Both phases parallelize — but only across commands that write **disjoint** outputs — and a bare `wait` swallows child exit codes, so both phases below explicitly collect each job's status and abort ship if any failed.
3939

40-
**Phase A — regenerate the always-in-repo committed artifacts (parallel), then let step 7 stage whatever changed.** Regenerate only the generators whose inputs live entirely in this repo and that any ordinary code change can drift — `agent-stream-docs:generate` (derives from the provider model registry) and `skills:sync` (derives from `.agents/skills/**`). They write disjoint trees (`apps/docs/…/agent.mdx` vs the `.claude`/`.cursor` command projections), so they parallelize safely, and each is idempotent (a no-op when already in sync):
40+
**Phase A — regenerate the always-in-repo committed artifacts (parallel), then let step 7 stage whatever changed.** Regenerate only the generators whose inputs live entirely in this repo and that any ordinary code change can drift — `agent-stream-docs:generate` (derives from the provider model registry), `skills:sync` (derives from `.agents/skills/**`), and `docs-manifest:generate` (derives from the `apps/docs/content/docs` page tree, so adding, removing, or renaming a page drifts it). They write disjoint files (`apps/docs/…/agent.mdx`, the `.claude`/`.cursor` command projections, and `apps/sim/lib/copilot/generated/docs-manifest.ts`), so they parallelize safely, and each is idempotent (a no-op when already in sync):
4141
```bash
4242
rm -f /tmp/ship-gen-results
43-
for g in agent-stream-docs:generate skills:sync; do
43+
for g in agent-stream-docs:generate skills:sync docs-manifest:generate; do
4444
( bun run "$g" >"/tmp/ship-gen-${g//:/-}.log" 2>&1; echo "$? $g" >>/tmp/ship-gen-results ) &
4545
done
4646
wait

‎apps/docs/content/docs/cli/troubleshooting.mdx‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ sim --version
100100
Then upgrade with the package manager you installed it with — using a different
101101
one installs a second copy instead of replacing the executable on your `PATH`:
102102

103-
<Tabs items={['npm', 'pnpm', 'bun']}>
103+
<Tabs items={['npm', 'pnpm', 'bun', 'yarn']}>
104104
<Tab value="npm">
105105
```bash
106106
npm install -g sim@latest
@@ -116,6 +116,11 @@ one installs a second copy instead of replacing the executable on your `PATH`:
116116
bun add -g sim@latest
117117
```
118118
</Tab>
119+
<Tab value="yarn">
120+
```bash
121+
yarn global add sim@latest
122+
```
123+
</Tab>
119124
</Tabs>
120125

121126
The CLI can also tell you this through a cached daily check on eligible

‎apps/docs/content/docs/platform/self-hosting/redis.mdx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ Sim uses Redis as a message bus and shared cache. Both deployments ship it by de
2424
With more than one app or realtime replica and no `REDIS_URL`, users on different pods stop seeing each other's edits and live status updates. Beyond one startup log line noting single-pod mode, nothing is logged — the app looks healthy and quietly loses events. Treat Redis as mandatory the moment `replicaCount` exceeds 1.
2525
</Callout>
2626

27-
Everything Sim keeps in Redis is cache, coordination state, or an in-flight event — never committed data, which lives in PostgreSQL and object storage. Persistence is therefore not required. Losing or restarting the instance is not free, though: cancellation markers and the cross-pod half of execution streaming live here, so active runs stop streaming and a cancellation issued across the gap may not land. Completed work is unaffected.
27+
Everything Sim keeps in Redis is cache, coordination state, or an in-flight event — never committed data, which lives in PostgreSQL and object storage. Persistence is therefore not required. Losing or restarting the instance is not free, though: cancellation markers and the cross-pod half of execution streaming live here, so active runs stop streaming and a cancellation issued across the gap may not land.
28+
29+
One exception is worth planning for. High-volume webhook triggers claim their idempotency keys in Redis by default, trading a narrow durability window for the throughput. A restart drops those claims, so a provider that retries a webhook Sim already finished can have it execute a second time. Flows where a repeat is not acceptable — anything touching money, billing, or compliance — should claim in PostgreSQL instead, which fate-shares the claim with the transaction; see `forceStorage: 'database'` on the idempotency service. Enabling Redis persistence narrows the window but does not close it.
2830

2931
## Configuration
3032

‎apps/sim/app/api/knowledge/member-connectors/route.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ export const GET = defineInternalJsonRoute({
1313
auth: internalSessionAuth,
1414
operation: knowledgeOperations.listWorkspaceMemberConnectors,
1515
rateLimit: internalRateLimits.none({ reason: 'One small read per visit to the Search tab' }),
16-
errorPolicy: internalKnowledgeErrorPolicies.connectors,
16+
errorPolicy: internalKnowledgeErrorPolicies.memberConnectors,
1717
mapInput: ({ query }) => ({ workspaceId: query.workspaceId }),
1818
useCase: listWorkspaceMemberConnectors,
1919
present: ({ connectors }) => ({ success: true as const, data: connectors }),

‎apps/sim/app/workspace/[workspaceId]/home/home.tsx‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -543,23 +543,26 @@ export function Home({ chatId, userName, userId }: HomeProps) {
543543
]
544544
)
545545

546+
/** An emptied search box returns to the sources; a send in any other mode has no search to clear. */
547+
const clearSearch = useCallback(() => {
548+
if (searchQueryValue !== null) setSearchQuery('')
549+
}, [searchQueryValue, setSearchQuery])
550+
546551
/**
547552
* A queued message re-enters the composer in the mode it was written in: an
548553
* Assistant question edits as an Assistant question, and never as a Search,
549-
* which submits nothing and would leave the edit stranded.
554+
* which submits nothing and would leave the edit stranded. Leaving Search
555+
* drops the query as it does everywhere else; a live one would put the
556+
* composer straight back into Search and strand the edit anyway.
550557
*/
551558
const restoreQueuedMode = useCallback(
552559
(requestMode: QueuedMessage['requestMode']) => {
560+
clearSearch()
553561
void setComposerMode(requestMode === 'ask' ? 'assistant' : 'build')
554562
},
555-
[setComposerMode]
563+
[clearSearch, setComposerMode]
556564
)
557565

558-
/** An emptied search box returns to the sources; a send in any other mode has no search to clear. */
559-
const clearSearch = useCallback(() => {
560-
if (searchQueryValue !== null) setSearchQuery('')
561-
}, [searchQueryValue, setSearchQuery])
562-
563566
/**
564567
* Summarize or Answer on a result: switch to Assistant and hand the question
565568
* to it. The submit reads the mode from this render, so it is sent as an

‎apps/sim/lib/knowledge/api/route-policies.test.ts‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,10 @@ import {
1212
WorkspaceApiKeyScopeAuthorizationError,
1313
} from '@/lib/core/application'
1414
import { OrchestrationError } from '@/lib/core/orchestration/types'
15-
import { v2KnowledgeErrorPolicies } from '@/lib/knowledge/api/route-policies'
15+
import {
16+
internalKnowledgeErrorPolicies,
17+
v2KnowledgeErrorPolicies,
18+
} from '@/lib/knowledge/api/route-policies'
1619

1720
describe('v2 knowledge error policies', () => {
1821
it.each([
@@ -70,3 +73,21 @@ describe('v2 knowledge error policies', () => {
7073
})
7174
})
7275
})
76+
77+
describe('internal knowledge error policies', () => {
78+
it('conceals a knowledge-base-scoped connector authorization failure', () => {
79+
const projected = internalKnowledgeErrorPolicies.connectors.project(
80+
new NoWorkspaceAccessError()
81+
)
82+
expect(projected?.status).toBe(404)
83+
expect(projected?.body).toMatchObject({ error: 'Knowledge base not found' })
84+
})
85+
86+
it('does not conceal the workspace-scoped member-connector listing', () => {
87+
const projected = internalKnowledgeErrorPolicies.memberConnectors.project(
88+
new NoWorkspaceAccessError()
89+
)
90+
expect(projected?.status).not.toBe(404)
91+
expect(JSON.stringify(projected?.body)).not.toContain('Knowledge base not found')
92+
})
93+
})

‎apps/sim/lib/knowledge/api/route-policies.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,8 @@ export const internalKnowledgeErrorPolicies = {
110110
internalKnowledgeErrorPolicy('Failed to process knowledge tag request')
111111
),
112112
connectors: concealKnowledgeBase(internalKnowledgeErrorPolicy('Internal server error')),
113+
/** Workspace-scoped like the bulk routes above, so likewise not concealed. */
114+
memberConnectors: internalKnowledgeErrorPolicy('Failed to fetch member connectors'),
113115
uploads: concealKnowledgeBase(internalKnowledgeUploadErrorPolicy),
114116
} as const
115117

0 commit comments

Comments
 (0)