diff --git a/CLAUDE.md b/CLAUDE.md index 08bbb5c..f418cb1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -44,3 +44,11 @@ camelCase is the repo-wide Python convention above. Two external interfaces do N TOML config keys stay camelCase, matching the repo style. Keep the snake_case-to-camelCase mapping explicit in `server.py`. Neither convention leaks into the other. + +## Commits + +- Plain messages only. Do NOT add a `Co-Authored-By` or `Generated with` trailer unless explicitly asked. +- Short subject line expressing what was done as a short imperative. +- Subject line only. No body, no additional text. +- Never commit unprompted. Verify (compile and test), report ready for review, then wait for review. +- When presenting code for review, use the `commit-message` skill to draft the commit subject alongside it. diff --git a/README.md b/README.md index 9fbe3db..ece58c8 100644 --- a/README.md +++ b/README.md @@ -87,6 +87,7 @@ A ticket id is the command: ```bash docket CORE-14 # show it, dependency context and all docket CORE-14 status # bare word, for a pipe +docket CORE-14 ready # true or false. Every dependency done? docket CORE-14 done # todo, wip, or done. The file follows docket CORE-14 set [-t TEXT] [-p N] [-r A,B|none] [-ra A,B] [-rr A,B] docket CORE-14 meta [KEY [VALUE]] [-c] @@ -96,7 +97,7 @@ Everything else works on the set: ```bash docket new CORE "Skirmish setup" [-r CORE-9,GEN-3] [-p 1] [-b TEXT] -docket list [-s todo] [-k CORE] [-m 2] +docket list [-s todo] [-k CORE] [-m 2] [-r] docket graph [-i CORE-14 | -k GEN] [-o FILE] docket key list | add KEY "desc" [-r TEXT] | remove KEY docket validate | deploy PATH | upgrade PATH @@ -104,16 +105,19 @@ docket validate | deploy PATH | upgrade PATH `-r` replaces the dependency list. `-ra` and `-rr` edit the one already there. Both in one call is refused. +A ticket is ready when every id in its `requires` names a ticket that is `done`. A missing dependency blocks, and a `done` ticket is never ready, so `docket list -r` is the set you can pick up right now. + Every short flag has a long form (`-k/--key`, `-p/--priority`, `-m/--priority-max`, and so on). `--help` lists your actual keys and priority range. ## MCP -`docket-mcp` is a stdio server. Ten tools, each returning JSON as text. +`docket-mcp` is a stdio server. Eleven tools, each returning JSON as text. | Tool | Purpose | |---|---| | `list_tickets(status?, key?, priority_max?)` | Summaries only, never bodies. | | `read_ticket(id)` | Full body plus both dependency directions. | +| `check_ready(id)` | Whether every dependency is done, and what is blocking. | | `create_ticket(key, title, body?, requires?, priority?)` | Allocates the id, writes the file. | | `update_ticket(id, title?, priority?, requires?, requires_add?, requires_remove?)` | Those three fields only. | | `set_status(id, status)` | Writes frontmatter and moves the file together. | diff --git a/docs/tickets/CLAUDE.md b/docs/tickets/CLAUDE.md index 089577d..a27b344 100644 --- a/docs/tickets/CLAUDE.md +++ b/docs/tickets/CLAUDE.md @@ -59,6 +59,7 @@ Filenames are frozen at creation. Retitling a ticket deliberately does not renam |---|---| | See what exists | `list_tickets` | | Read one ticket in full | `read_ticket` | +| Check a ticket can be worked on | `check_ready` | | Create a ticket | `create_ticket` | | See the dependency graph | `graph` | | See valid keys | `list_keys` | @@ -71,6 +72,8 @@ A ticket declares what it `requires`. It never declares what it blocks. The reverse direction is derived, not stored. `read_ticket` returns both, so to find out what a ticket is blocking, read it and look at `requiredBy`. Do not add a "blocks" field. Storing both directions guarantees they eventually disagree, which is exactly what this design exists to prevent. +Whether a ticket is ready to be worked on is derived the same way. Call `check_ready` rather than listing statuses and deciding for yourself, so that every caller gets the same answer from the same rule. Ready means every id in `requires` names a ticket that is `done`, a missing dependency blocks, and a ticket that is already `done` is never ready. + To change one edge, use `update_ticket` with `requires_add` or `requires_remove` rather than reading the list and passing it back with one entry different. Those edit the list in place, so nothing you did not name is at risk. Reserve `requires` for when you genuinely mean to replace the whole list, and never pass it in the same call as an edit, which is refused. ## Keys are closed diff --git a/src/docket/__init__.py b/src/docket/__init__.py index 47869b7..a19a5d7 100644 --- a/src/docket/__init__.py +++ b/src/docket/__init__.py @@ -8,4 +8,4 @@ # No type check to comply with hatch's requirements. # Do not re-add. -__version__ = "1.0.1" +__version__ = "1.1.0" diff --git a/src/docket/cli/commands.py b/src/docket/cli/commands.py index cc73779..d2aae30 100644 --- a/src/docket/cli/commands.py +++ b/src/docket/cli/commands.py @@ -18,7 +18,7 @@ from docket.cli.output import STATUS_STYLES, Output, buildContextTable, relativeToRoot from docket.core.config import Config from docket.core.deploy import DeployReport, deploy, upgrade -from docket.core.graph import ResolvedGraph, dependencyContext, resolveGraph, subgraphForId, subgraphForKey +from docket.core.graph import Readiness, ResolvedGraph, dependencyContext, readyTickets, resolveGraph, subgraphForId, subgraphForKey, ticketReadiness from docket.core.inputs import requireWritableFile, writeFile from docket.core.mermaid import renderGraph from docket.core.store import Store, TicketResult, TicketSet @@ -49,6 +49,7 @@ def commandTicket(args: argparse.Namespace, store: Store, output: Output) -> int None: commandShow, "show": commandShow, "status": commandStatusRead, + "ready": commandReady, "set": commandSet, "meta": commandMeta, } @@ -133,7 +134,12 @@ def commandList(args: argparse.Namespace, store: Store, output: Output) -> int: if key is not None: store.config.requireKnownKey(key) - tickets: list[Ticket] = store.loadAll().filtered(status=status, key=key, priorityMax=priorityMax) + loaded: TicketSet = store.loadAll() + tickets: list[Ticket] = loaded.filtered(status=status, key=key, priorityMax=priorityMax) + + # Readiness is judged against the whole set rather than the narrowed one, since a dependency may well have been filtered out of the listing. + if args.ready: + tickets = readyTickets(loaded, tickets) if not tickets: output.print("[dim]No tickets matched.[/dim]") @@ -225,6 +231,26 @@ def commandStatusRead(args: argparse.Namespace, store: Store, output: Output) -> return EXIT_OK +def commandReady(args: argparse.Namespace, store: Store, output: Output) -> int: + """ + Print whether a ticket's dependencies are all done, and nothing else. + + This goes out raw for the same reason `status` does. What is blocking is deliberately left to `show`, which already tables both dependency directions with their statuses. + + args: The parsed arguments. + store: The store to read from. + output: Where to write. + + Returns the process exit code, which reports whether the question could be answered rather than what the answer was. + """ + + readiness: Readiness = ticketReadiness(store.loadAll(), args.id) + + output.raw(f"{'true' if readiness.isReady else 'false'}\n") + + return EXIT_OK + + def commandMeta(args: argparse.Namespace, store: Store, output: Output) -> int: """ Inspect and manage a ticket's metadata map. diff --git a/src/docket/cli/grammar.py b/src/docket/cli/grammar.py index 900117c..d643ef6 100644 --- a/src/docket/cli/grammar.py +++ b/src/docket/cli/grammar.py @@ -251,6 +251,7 @@ def buildParser(config: Optional[Config] = None) -> argparse.ArgumentParser: listParser.add_argument("-s", "--status", choices=STATUSES, help="Keep only tickets with this status.") listParser.add_argument("-k", "--key", help=f"Keep only tickets carrying this key. {keyOptions}") listParser.add_argument("-m", "--priority-max", type=int, dest="priorityMax", help=f"Keep only tickets at or below this priority number. {priorityOptions}") + listParser.add_argument("-r", "--ready", action="store_true", help="Keep only tickets whose dependencies are all done. A done ticket is never ready, so this never shows one.") graphParser: argparse.ArgumentParser = commands.add_parser("graph", help="Render the dependency graph as mermaid source.", formatter_class=RichHelpFormatter) graphParser.add_argument("scope", nargs="?", metavar="SCOPE", help="What to scope to, read from its own shape: a ticket id, or a key. The flags below are the same two, named explicitly.") @@ -309,6 +310,7 @@ def buildTicketParser(commands: argparse._SubParsersAction, priorityOptions: str ticketCommands.add_parser("show", help="Show the ticket with its resolved dependency context. This is what a bare id does.", formatter_class=RichHelpFormatter) ticketCommands.add_parser("status", help="Print the ticket's status and nothing else, for a pipe to read.", formatter_class=RichHelpFormatter) + ticketCommands.add_parser("ready", help="Print whether every dependency is done, as a bare true or false, for a pipe to read.", formatter_class=RichHelpFormatter) # One parser per status is what makes 'docket CORE-14 done' work. It also puts the whole vocabulary into the error when a command is misspelled, which a single `choices` list on a value argument could not do. for status in STATUSES: diff --git a/src/docket/core/graph.py b/src/docket/core/graph.py index 69983d8..2a8198a 100644 --- a/src/docket/core/graph.py +++ b/src/docket/core/graph.py @@ -58,6 +58,25 @@ class Edge: toId: str +@dataclass(frozen=True) +class Readiness: + """ + Whether one ticket can be worked on now, and what stands in the way when it cannot. + + This is derived from the set on every read and never stored, for the same reason a reverse edge is. A stored answer would go stale the moment a dependency was closed. + """ + + # MARK: Properties + + id: str + + # Whether every dependency is done and there is still work left to do. + isReady: bool + + # The resolved records of everything holding this ticket back, in the same shape `dependencyContext` returns. Empty on a ticket that is itself done, since nothing is blocking it. + blockedBy: tuple[dict[str, object], ...] = () + + @dataclass class ResolvedGraph: """ @@ -230,14 +249,47 @@ def dependencyContext(ticketSet: TicketSet, ticketId: str) -> dict[str, list[dic # A dependency naming a missing id still has to appear, since hiding it would make a broken link invisible to the reader. requires: list[dict[str, object]] = [] for requiredId in ticket.requires: - requires.append(_contextEntry(graph, requiredId)) + requires.append(_contextEntry(ticketSet, requiredId)) + # The reverse direction is the one thing here the set alone cannot answer, which is what the graph is resolved for. node: Optional[GraphNode] = graph.nodes.get(ticketId) - requiredBy: list[dict[str, object]] = [_contextEntry(graph, dependentId) for dependentId in (node.requiredBy if node is not None else ())] + requiredBy: list[dict[str, object]] = [_contextEntry(ticketSet, dependentId) for dependentId in (node.requiredBy if node is not None else ())] return {"requires": requires, "requiredBy": requiredBy} +def ticketReadiness(ticketSet: TicketSet, ticketId: str) -> Readiness: + """ + Decide whether one ticket can be worked on now. + + Ready means every id in `requires` names a ticket that is done. Only the direct dependencies are consulted, because a done ticket is taken at its word. A done dependency with unfinished dependencies of its own is an inconsistency, and reporting that is `validate`'s job rather than this one's. + + A dependency naming a ticket that does not exist blocks, since a link the reader cannot follow is not the same as clear road. A cycle blocks without a special case, because no ticket in one is ever done. + + ticketSet: The loaded tickets. + ticketId: The ticket to judge. + + Returns the readiness of that ticket. + """ + + return _readinessOf(ticketSet, ticketSet.get(ticketId)) + + +def readyTickets(ticketSet: TicketSet, tickets: Iterable[Ticket]) -> list[Ticket]: + """ + Select the tickets that can be worked on now, keeping the order they arrived in. + + The whole set is needed to judge any one ticket, so the candidates are passed separately from the set they are judged against. That is what lets a caller filter an already narrowed listing. + + ticketSet: The loaded tickets, which every dependency is looked up in. + tickets: The candidates to filter. + + Returns the ready candidates. + """ + + return [ticket for ticket in tickets if _readinessOf(ticketSet, ticket).isReady] + + def findCycles(graph: ResolvedGraph) -> list[list[str]]: """ Find every dependency cycle, reporting one representative per strongly connected component. @@ -312,21 +364,54 @@ def findCycles(graph: ResolvedGraph) -> list[list[str]]: return sorted(cycles) -def _contextEntry(graph: ResolvedGraph, ticketId: str) -> dict[str, object]: +def _readinessOf(ticketSet: TicketSet, ticket: Ticket) -> Readiness: + """ + Judge one already-loaded ticket, which is what both public entry points do their work through. + + ticketSet: The loaded tickets, which every dependency is looked up in. + ticket: The ticket to judge. + + Returns the readiness of that ticket. + """ + + # A finished ticket has no work left to be ready for, so it is not ready and nothing is holding it back. + if ticket.isDone: + return Readiness(id=ticket.id, isReady=False) + + blockers: list[dict[str, object]] = [_contextEntry(ticketSet, requiredId) for requiredId in ticket.requires if not _isSatisfied(ticketSet, requiredId)] + + return Readiness(id=ticket.id, isReady=not blockers, blockedBy=tuple(blockers)) + + +def _isSatisfied(ticketSet: TicketSet, requiredId: str) -> bool: + """ + Report whether one dependency is met. + + requiredId: The id the depending ticket names. + + Returns `True` only when the id names a ticket that exists and is done. + """ + + required: Optional[Ticket] = ticketSet.tickets.get(requiredId) + + return required is not None and required.isDone + + +def _contextEntry(ticketSet: TicketSet, ticketId: str) -> dict[str, object]: """ Build one resolved dependency record. - graph: The resolved graph to look the ticket up in. + ticketSet: The loaded tickets to look the ticket up in. ticketId: The id to describe. Returns the record, flagged when the id names nothing that exists. """ - node: Optional[GraphNode] = graph.nodes.get(ticketId) - if node is None: + ticket: Optional[Ticket] = ticketSet.tickets.get(ticketId) + if ticket is None: return {"id": ticketId, "title": None, "status": None, "priority": None, "exists": False} - return {"id": node.id, "title": node.title, "status": node.status, "priority": node.priority, "exists": True} + return {"id": ticket.id, "title": ticket.title, "status": ticket.status, "priority": ticket.priority, "exists": True} def _reachable(graph: ResolvedGraph, startId: str, forward: bool) -> set[str]: diff --git a/src/docket/server.py b/src/docket/server.py index 87ddd37..f4162d7 100644 --- a/src/docket/server.py +++ b/src/docket/server.py @@ -32,7 +32,7 @@ from docket import __version__ from docket.core.config import Config, discoverConfig -from docket.core.graph import ResolvedGraph, dependencyContext, resolveGraph, subgraphForId, subgraphForKey +from docket.core.graph import Readiness, ResolvedGraph, dependencyContext, resolveGraph, subgraphForId, subgraphForKey, ticketReadiness from docket.core.mermaid import renderGraph from docket.core.store import Store, TicketResult, TicketSet from docket.core.ticket import Ticket @@ -55,6 +55,8 @@ Dependencies are stored in one direction only. A ticket declares `requires`, and never declares what it blocks. `read_ticket` returns both directions, deriving the reverse side for you. +Whether a ticket can be worked on now is `check_ready`, and reading it off a status list yourself is not. The rule lives in one place so that every caller gets the same answer. + Keys are closed. Call `list_keys` before `create_ticket`. When no existing key fits, ask the user with `AskUserQuestion` whether to add one, and call `add_key` only once they agree. """ @@ -101,6 +103,7 @@ async def readTicket(id: str) -> str: payload: dict[str, Any] = dict(ticket.summary()) payload["body"] = ticket.body + payload["ready"] = ticketReadiness(loaded, id).isReady payload["requires"] = context["requires"] payload["requiredBy"] = context["requiredBy"] payload["metadata"] = ticket.metadata @@ -109,6 +112,24 @@ async def readTicket(id: str) -> str: return _json(payload) +@mcp.tool(name="check_ready") +async def checkReady(id: str) -> str: + """ + Report whether a ticket can be worked on now, rather than leaving you to work it out. + + Ready means every id in `requires` names a ticket that is done. Only the direct dependencies are consulted, since a done ticket is taken at its word. A dependency naming a ticket that does not exist blocks, and is returned in `blocked_by` with `exists` false. + + A ticket that is itself done is never ready, because there is no work left to be ready for. That case returns an empty `blocked_by`, so an empty list alongside `ready` false means finished rather than unblocked. + + id: The ticket id, for example CORE-14. + """ + + store: Store = _store() + readiness: Readiness = ticketReadiness(store.loadAll(), id) + + return _json({"id": readiness.id, "ready": readiness.isReady, "blocked_by": list(readiness.blockedBy)}) + + @mcp.tool(name="create_ticket") async def createTicket( key: str, diff --git a/src/docket/templates/CLAUDE.md b/src/docket/templates/CLAUDE.md index 089577d..a27b344 100644 --- a/src/docket/templates/CLAUDE.md +++ b/src/docket/templates/CLAUDE.md @@ -59,6 +59,7 @@ Filenames are frozen at creation. Retitling a ticket deliberately does not renam |---|---| | See what exists | `list_tickets` | | Read one ticket in full | `read_ticket` | +| Check a ticket can be worked on | `check_ready` | | Create a ticket | `create_ticket` | | See the dependency graph | `graph` | | See valid keys | `list_keys` | @@ -71,6 +72,8 @@ A ticket declares what it `requires`. It never declares what it blocks. The reverse direction is derived, not stored. `read_ticket` returns both, so to find out what a ticket is blocking, read it and look at `requiredBy`. Do not add a "blocks" field. Storing both directions guarantees they eventually disagree, which is exactly what this design exists to prevent. +Whether a ticket is ready to be worked on is derived the same way. Call `check_ready` rather than listing statuses and deciding for yourself, so that every caller gets the same answer from the same rule. Ready means every id in `requires` names a ticket that is `done`, a missing dependency blocks, and a ticket that is already `done` is never ready. + To change one edge, use `update_ticket` with `requires_add` or `requires_remove` rather than reading the list and passing it back with one entry different. Those edit the list in place, so nothing you did not name is at risk. Reserve `requires` for when you genuinely mean to replace the whole list, and never pass it in the same call as an edit, which is refused. ## Keys are closed diff --git a/tests/test_cli.py b/tests/test_cli.py index f0a5951..e872304 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -657,6 +657,112 @@ def testStatusPrintsOnlyTheStatus(inRepo: Path, capsys: pytest.CaptureFixture[st assert "\x1b" not in out +def testReadyPrintsOnlyTheAnswer(inRepo: Path, capsys: pytest.CaptureFixture[str]) -> None: + """ + Readiness reads bare for the same reason a status does, so a shell can take the answer as readily as a person. + """ + + main(["new", "CORE", "Skirmish setup"]) + capsys.readouterr() + + assert main(["CORE-1", "ready"]) == EXIT_OK + + out: str = capsys.readouterr().out + + assert out == "true\n" + assert "\x1b" not in out + + +def testReadyIsFalseWhileADependencyIsOpen(inRepo: Path, capsys: pytest.CaptureFixture[str]) -> None: + """ + An open prerequisite is the case the check exists for, and closing it has to flip the answer. + """ + + main(["new", "CORE", "Skirmish setup"]) + main(["new", "CORE", "Deployment", "--requires", "CORE-1"]) + capsys.readouterr() + + assert main(["CORE-2", "ready"]) == EXIT_OK + assert capsys.readouterr().out == "false\n" + + main(["CORE-1", "done"]) + capsys.readouterr() + + assert main(["CORE-2", "ready"]) == EXIT_OK + assert capsys.readouterr().out == "true\n" + + +def testReadyExitsZeroWhenNotReady(inRepo: Path, capsys: pytest.CaptureFixture[str]) -> None: + """ + The exit code reports whether the question could be answered, not what the answer was, so a false is a successful read rather than a failure. + """ + + main(["new", "CORE", "Skirmish setup"]) + main(["new", "CORE", "Deployment", "--requires", "CORE-1"]) + capsys.readouterr() + + assert main(["CORE-2", "ready"]) == EXIT_OK + + +def testReadyOnAnUnknownTicketFails(inRepo: Path, capsys: pytest.CaptureFixture[str]) -> None: + """ + A ticket that does not exist has no readiness to report, so it fails the way naming an unknown ticket always does rather than answering false. + """ + + assert main(["CORE-99", "ready"]) == EXIT_USAGE + assert "CORE-99" in capsys.readouterr().err + + +def testListReadyKeepsOnlyUnblockedTickets(inRepo: Path, capsys: pytest.CaptureFixture[str]) -> None: + """ + The filter answers "what can I pick up right now", so a blocked ticket and a finished one both fall out of it. + """ + + main(["new", "CORE", "Skirmish setup"]) + main(["new", "CORE", "Deployment", "--requires", "CORE-1"]) + main(["new", "CORE", "Shipped"]) + main(["CORE-3", "done"]) + capsys.readouterr() + + assert main(["list", "--ready"]) == EXIT_OK + + out: str = capsys.readouterr().out + + assert "Skirmish setup" in out + assert "Deployment" not in out + assert "Shipped" not in out + + +def testListReadyComposesWithTheOtherFilters(inRepo: Path, capsys: pytest.CaptureFixture[str]) -> None: + """ + Readiness narrows what the other filters already selected rather than replacing them. + """ + + main(["new", "CORE", "Core work"]) + main(["new", "GEN", "Gen work"]) + capsys.readouterr() + + assert main(["list", "CORE", "--ready"]) == EXIT_OK + + out: str = capsys.readouterr().out + + assert "Core work" in out + assert "Gen work" not in out + + +def testListReadyJudgesAgainstTheWholeSet(inRepo: Path, capsys: pytest.CaptureFixture[str]) -> None: + """ + A dependency filtered out of the listing still counts, since readiness is a fact about the set rather than about what was displayed. + """ + + main(["new", "GEN", "Groundwork"]) + main(["new", "CORE", "Skirmish setup", "--requires", "GEN-1"]) + capsys.readouterr() + + assert main(["list", "CORE", "--ready"]) == EXIT_OK + assert "No tickets matched" in capsys.readouterr().out + + def testAnUnknownStatusIsRejectedAtTheParser(inRepo: Path) -> None: """ The vocabulary is fixed, so argparse refuses an unrecognized word before the core is reached. diff --git a/tests/test_graph.py b/tests/test_graph.py index b5a45a2..0bbeaf9 100644 --- a/tests/test_graph.py +++ b/tests/test_graph.py @@ -1,14 +1,14 @@ """ Graph Tests -Cover reverse edge derivation, scoped traversal, dependency context, and cycle detection. +Cover reverse edge derivation, scoped traversal, dependency context, readiness, and cycle detection. """ # MARK: Imports import pytest -from docket.core.graph import Edge, ResolvedGraph, dependencyContext, findCycles, resolveGraph, subgraphForId, subgraphForKey +from docket.core.graph import Edge, Readiness, ResolvedGraph, dependencyContext, findCycles, readyTickets, resolveGraph, subgraphForId, subgraphForKey, ticketReadiness from docket.core.store import TicketSet from docket.core.ticket import Ticket @@ -31,6 +31,23 @@ def buildSet(*specs: tuple[str, list[str]]) -> TicketSet: return ticketSet +def buildStatusSet(*specs: tuple[str, str, list[str]]) -> TicketSet: + """ + Build a ticket set whose statuses differ, which is what every readiness case turns on. + + specs: Triples of a ticket id, its status, and the ids it requires. + + Returns the assembled set. + """ + + ticketSet: TicketSet = buildSet(*[(ticketId, requires) for ticketId, _, requires in specs]) + + for ticketId, status, _ in specs: + ticketSet.tickets[ticketId].status = status + + return ticketSet + + def testReverseEdgesAreDerived() -> None: """ A ticket never stores what it blocks, so the reverse direction has to come back from a scan. @@ -193,6 +210,109 @@ def testDependencyContextReportsAMissingDependency() -> None: assert context["requires"] == [{"id": "CORE-99", "title": None, "status": None, "priority": None, "exists": False}] +def testATicketWithNoDependenciesIsReady() -> None: + """ + Nothing to wait on is the common case, so it needs no special handling to come back ready. + """ + + assert ticketReadiness(buildSet(("CORE-1", [])), "CORE-1") == Readiness(id="CORE-1", isReady=True, blockedBy=()) + + +def testEveryDependencyDoneMakesATicketReady() -> None: + """ + Ready is the whole question the check exists to answer, and this is the shape of a yes. + """ + + ticketSet: TicketSet = buildStatusSet(("CORE-9", "done", []), ("GEN-3", "done", []), ("CORE-14", "todo", ["CORE-9", "GEN-3"])) + + assert ticketReadiness(ticketSet, "CORE-14").isReady + + +def testOneUnfinishedDependencyBlocks() -> None: + """ + A single open prerequisite is enough, and the answer has to name it rather than only refusing. + """ + + ticketSet: TicketSet = buildStatusSet(("CORE-9", "done", []), ("GEN-3", "wip", []), ("CORE-14", "todo", ["CORE-9", "GEN-3"])) + + readiness: Readiness = ticketReadiness(ticketSet, "CORE-14") + + assert not readiness.isReady + assert readiness.blockedBy == ({"id": "GEN-3", "title": "Title GEN-3", "status": "wip", "priority": 2, "exists": True},) + + +def testAMissingDependencyBlocks() -> None: + """ + A link the reader cannot follow is not the same as clear road, so it blocks and stays visible. + """ + + readiness: Readiness = ticketReadiness(buildSet(("CORE-14", ["CORE-99"])), "CORE-14") + + assert not readiness.isReady + assert readiness.blockedBy == ({"id": "CORE-99", "title": None, "status": None, "priority": None, "exists": False},) + + +def testAWipTicketCanStillBeReady() -> None: + """ + Readiness asks about the dependencies, so work already started does not change the answer. + """ + + assert ticketReadiness(buildStatusSet(("CORE-1", "wip", [])), "CORE-1").isReady + + +def testADoneTicketIsNeverReady() -> None: + """ + There is no work left to be ready for, and an empty blocker list is what tells the two apart from being unblocked. + """ + + readiness: Readiness = ticketReadiness(buildStatusSet(("CORE-9", "done", []), ("CORE-14", "done", ["CORE-9"])), "CORE-14") + + assert not readiness.isReady + assert readiness.blockedBy == () + + +def testOnlyDirectDependenciesAreConsulted() -> None: + """ + A done dependency is taken at its word, since an unfinished dependency behind it is `validate`'s finding rather than this rule's. + """ + + ticketSet: TicketSet = buildStatusSet(("CORE-1", "todo", []), ("CORE-9", "done", ["CORE-1"]), ("CORE-14", "todo", ["CORE-9"])) + + assert ticketReadiness(ticketSet, "CORE-14").isReady + + +def testACycleBlocksWithoutASpecialCase() -> None: + """ + No ticket in a cycle can be done, so each one blocks the next by the ordinary rule. + """ + + ticketSet: TicketSet = buildSet(("CORE-1", ["CORE-2"]), ("CORE-2", ["CORE-1"])) + + assert not ticketReadiness(ticketSet, "CORE-1").isReady + assert not ticketReadiness(ticketSet, "CORE-2").isReady + + +def testReadyTicketsFiltersAndKeepsOrder() -> None: + """ + A listing is already ordered by the time it is filtered, so the filter must not reorder what survives. + """ + + ticketSet: TicketSet = buildStatusSet(("CORE-9", "done", []), ("CORE-1", "todo", []), ("CORE-2", "todo", ["CORE-1"]), ("CORE-3", "todo", ["CORE-9"])) + candidates: list[Ticket] = [ticketSet.tickets[ticketId] for ticketId in ("CORE-3", "CORE-2", "CORE-1")] + + assert [ticket.id for ticket in readyTickets(ticketSet, candidates)] == ["CORE-3", "CORE-1"] + + +def testReadyTicketsJudgesAgainstTheWholeSet() -> None: + """ + A dependency may well have been filtered out of the listing, so the candidates and the set they are judged against are separate arguments. + """ + + ticketSet: TicketSet = buildStatusSet(("CORE-9", "done", []), ("CORE-14", "todo", ["CORE-9"])) + + assert [ticket.id for ticket in readyTickets(ticketSet, [ticketSet.tickets["CORE-14"]])] == ["CORE-14"] + + def testNoCyclesOnAnAcyclicGraph() -> None: """ A well-formed dependency chain reports nothing. diff --git a/tests/test_server.py b/tests/test_server.py index ce02b41..14153a1 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -89,7 +89,7 @@ def testEveryHandlerIsAsync() -> None: """ `MCPServer` runs a synchronous handler on a worker thread, which would let two calls into the core at once, and every write there reads the ticket set and then writes it back with nothing guarding the gap. - Two creates would allocate one id twice, two edits would lose one of them, and a read landing mid-write would see a half-written file. Staying on the event loop is what prevents all three, so it is asserted rather than left to whoever adds the eleventh tool. + Two creates would allocate one id twice, two edits would lose one of them, and a read landing mid-write would see a half-written file. Staying on the event loop is what prevents all three, so it is asserted rather than left to whoever adds the twelfth tool. """ handlers: list[Callable[..., Any]] = toolHandlers() @@ -103,11 +103,11 @@ def testEveryHandlerIsAsync() -> None: def testEveryDocumentedToolIsRegistered() -> None: """ - The surface is exactly the ten tools the design specifies, no more and no fewer. + The surface is exactly the eleven tools the design specifies, no more and no fewer. """ assert sorted(toolNames()) == sorted( - ["list_tickets", "read_ticket", "create_ticket", "update_ticket", "set_metadata", "set_status", "graph", "list_keys", "add_key", "validate"] + ["list_tickets", "read_ticket", "check_ready", "create_ticket", "update_ticket", "set_metadata", "set_status", "graph", "list_keys", "add_key", "validate"] ) @@ -281,6 +281,77 @@ def testReadTicketFlagsAMissingDependency(inRepo: Path) -> None: assert payload["requires"][0]["id"] == "CORE-99" +def testReadTicketCarriesReadiness(inRepo: Path) -> None: + """ + An agent already reading a ticket must not have to make a second call to find out whether it can act on it. + """ + + callTool("create_ticket", {"key": "CORE", "title": "App shell"}) + callTool("create_ticket", {"key": "CORE", "title": "Skirmish setup", "requires": ["CORE-1"]}) + + assert callTool("read_ticket", {"id": "CORE-1"})["ready"] is True + assert callTool("read_ticket", {"id": "CORE-2"})["ready"] is False + + +def testCheckReadyNamesWhatIsBlocking(inRepo: Path) -> None: + """ + Refusing without naming the blocker would leave the agent to work out the reason, which is the inference this tool exists to replace. + """ + + callTool("create_ticket", {"key": "CORE", "title": "App shell"}) + callTool("create_ticket", {"key": "CORE", "title": "Skirmish setup", "requires": ["CORE-1"]}) + + payload = callTool("check_ready", {"id": "CORE-2"}) + + assert payload == { + "id": "CORE-2", + "ready": False, + "blocked_by": [{"id": "CORE-1", "title": "App shell", "status": "todo", "priority": 2, "exists": True}], + } + + +def testCheckReadyClearsOnceTheDependencyIsDone(inRepo: Path) -> None: + """ + Readiness is derived on every read rather than stored, so closing a dependency flips the answer with nothing else written. + """ + + callTool("create_ticket", {"key": "CORE", "title": "App shell"}) + callTool("create_ticket", {"key": "CORE", "title": "Skirmish setup", "requires": ["CORE-1"]}) + callTool("set_status", {"id": "CORE-1", "status": "done"}) + + payload = callTool("check_ready", {"id": "CORE-2"}) + + assert payload["ready"] is True + assert payload["blocked_by"] == [] + + +def testCheckReadyFlagsAMissingDependency(inRepo: Path) -> None: + """ + A link the agent cannot follow blocks, and it is returned marked rather than reported as a plain refusal. + """ + + callTool("create_ticket", {"key": "CORE", "title": "Dangling", "requires": ["CORE-99"]}) + + payload = callTool("check_ready", {"id": "CORE-1"}) + + assert payload["ready"] is False + assert payload["blocked_by"] == [{"id": "CORE-99", "title": None, "status": None, "priority": None, "exists": False}] + + +def testCheckReadyOnADoneTicketReturnsNoBlockers(inRepo: Path) -> None: + """ + A finished ticket is not ready and nothing is blocking it, which is the one case where an empty blocker list means finished rather than unblocked. + """ + + callTool("create_ticket", {"key": "CORE", "title": "Shipped"}) + callTool("set_status", {"id": "CORE-1", "status": "done"}) + + payload = callTool("check_ready", {"id": "CORE-1"}) + + assert payload["ready"] is False + assert payload["blocked_by"] == [] + + def testReadTicketCarriesUnknownFields(inRepo: Path) -> None: """ A consumer repository's own frontmatter fields are visible to the agent rather than silently dropped.