feat(services): rotating tips surface#281
Conversation
Adds TipsService that nudges the user toward features they may not know about — bundled markdown snippets that surface during idle moments at low frequency. Conservative defaults (one tip every five sessions, thirty-day repeat window), passive display only, and persistent per-user state for dismiss/snooze/last-shown. Ships fifteen bundled tips covering recently-landed features. The slash command surface is /tips, /tips dismiss <id>, /tips off, and /tips on. The REPL renders one tip per session, post-first-turn. Reuses the skills frontmatter parser via a new generic parse_frontmatter_into helper.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbf57a8cd2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let value = value.trim().trim_matches('"').trim_matches('\''); | ||
|
|
||
| // Handle booleans. | ||
| // Handle booleans and integers; everything else is a string. | ||
| let json_value = match value { | ||
| "true" => serde_json::Value::Bool(true), |
There was a problem hiding this comment.
Keep quoted numeric frontmatter values as strings
This parser now strips quotes and then coerces any numeric-looking scalar to i64, which changes skill frontmatter behavior: values like description: "2026" or whenToUse: "1" are converted to JSON numbers, then fail deserialization into Option<String>, causing parse_frontmatter to return Invalid frontmatter and skip loading that skill. Quoted scalars should stay strings (or coercion should be field-aware) to avoid breaking existing skills with numeric-looking text.
Useful? React with 👍 / 👎.
The hand-rolled YAML parser stripped outer quotes BEFORE checking for numeric coercion, so a skill or tip with `description: "2026"` ended up as the integer 2026 instead of the string "2026". Same class of bug for `userInvocable: "false"` becoming the bool false. This breaks compatibility with skills authored against any real YAML parser, where a quoted scalar is always a string regardless of contents. Track whether the value was quoted in the source. Quoted values short-circuit straight to JSON String. Only unquoted values go through bool/integer coercion.
Summary
TipsService(crates/lib/src/services/tips.rs) — a rotating tip surface with conservative defaults (1 tip every 5 sessions, 30-day repeat window) and persistent per-user state under<agent-config>/tips_state.json.config::atomic::atomic_write_secret./tips,/tips dismiss <id>,/tips off,/tips onslash commands and renders one tip per REPL session, post-first-turn.The frontmatter parser is reused —
crates/lib/src/skills/mod.rsexposes a new genericparse_frontmatter_into<T>so tips and any future bundled-markdown subsystem deserialize against their own metadata schema without duplicating the splitter or YAML reader.Bundled tips
skills-list— listing and invoking skillsmodel-tools— Brief / Config / McpAuth model-callable toolsoutput-style-themes—/output-styleplus colorblind / ANSI-only themesteam-memory— team-memory layer +/team-remembercron-schedule—/schedule create,/schedule listagent-worktrees— Agent tool withisolation: "worktree"plan-mode— read-only exploration before actionmulti-edit— batch file changessyntax-theme— Ctrl+T to flip syntax themereload— re-reads disk styles + memoryinherit-fg—[ui].inherit_fgfor terminal-native foregroundbundled-skills—batch,loop,simplify,verify,stuck,remember,app-builderplugin-marketplace—/pluginmarketplacetips-off—/tips offto suppressremote-trigger—RemoteTriggertoolTips are passive; the prompt loop is never blocked. The REPL skips rendering when in non-TTY,
--prompt,--serve, or--acpmode by virtue of those paths never reaching the REPL.Test plan
cargo check --all-targetscargo test --all-targets— 1116 lib + 249 cli + integration tests pass; pre-existingbwrap_*sandbox failures (require user-namespace privileges) are not from this change.cargo clippy --all-targets -- -D warningscargo fmt --all -- --checkservices::tips::tests: 15-tip parse, no third-party attribution in bodies, dismiss + persistence round-trip, snooze respected, weighted ratio (1:9 over 1000 iterations), 30-day repeat-window enforcement, exhaust + reset, cadence gate, disabled gate,show_after_sessiongate.commands::testsexercise/tips,/tips dismiss,/tips off,/tips on, unknown subcommands against a tempdir-backed service.